OpenAPI: Deprecate oauth/tokens endpoint#10603
Conversation
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
16e4a1e to
2efb0d4
Compare
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
2efb0d4 to
38cf158
Compare
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
38cf158 to
9e100ba
Compare
snazy
force-pushed
the
rest-tokens-deprecate
branch
2 times, most recently
from
48fc819 to
cc10a13
Compare
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
Outdated
Show resolved
Hide resolved
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
4bc026c to
b710286
Compare
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
196f50c to
66fddb3
Compare
snazy
force-pushed
the
rest-tokens-deprecate
branch
2 times, most recently
from
0a1b94f to
6fc0f69
Compare
open-api/rest-catalog-open-api.yaml
Outdated
| to the correct OAuth endpoint. | ||
|
|
||
| Deprecated since Iceberg (Java) 1.6.0. The endpoint and related types will be removed from | ||
| this spec in Iceberg (Java) 1.7.0. |
There was a problem hiding this comment.
what's the implication of removing this (and all the request / response types) from the Spec in 1.7.0 but not actually from the implementation? To me it seems that this should be marked for removal with Iceberg 2.0
There was a problem hiding this comment.
It's to let (new) adopters run into the trap of "blindly" implementing it and accidentally run into security issues
There was a problem hiding this comment.
Removing the endpoint from the OpenAPI YAML could break auto-generated clients. Custom clients (e.g. the Iceberg java REST client) calling this endpoint in servers that offer backward compatibility will not be affected. AFAIK, PyIceberg also not affected by dropping the endpoint from OpenAPI.
Given the discussion of the negative security aspects of this endpoint (in the dev mail list), I tend to think that removing the endpoint from Open API sooner (1.7.0) is worth the potential hardship for auto-generated clients.
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
6fc0f69 to
27dd00f
Compare
|
I have added 1.6.0 milestone for this now because we still have a day or two for the release cut and we have mentioned that we are deprecating it in 1.6.0 in the PR changes. |
nastra
changed the title
oauth/tokens endpointoauth/tokens endpoint
|
I've updated the PR to mention "2.0". The CI failures look unrelated, but I don't have the power to rerun those. |
|
Can you try rebase to see if it fixes the CI? |
snazy
force-pushed
the
rest-tokens-deprecate
branch
from
2b566d1 to
a1e3c73
Compare
CI looking good |
|
Thanks everyone, moving this forward for the 1.6.0 release 👍 |
* Deprecate `oauth/tokens` endpoint This PR implements "M1" of [this document](https://docs.google.com/document/d/1Xi5MRk8WdBWFC3N_eSmVcrLhk3yu5nJ9x_wC0ec6kVQ/), see apache#10537. * update wording in spec * 2 * left-over
* Deprecate `oauth/tokens` endpoint This PR implements "M1" of [this document](https://docs.google.com/document/d/1Xi5MRk8WdBWFC3N_eSmVcrLhk3yu5nJ9x_wC0ec6kVQ/), see apache#10537. * update wording in spec * 2 * left-over
8 participants
This PR implements "M1" of this document, see #10537.