Skip to content

[FLINK-30370][runtime][security] Move existing delegation token framework authentication to providers#21499

Merged
mbalassi merged 1 commit intoapache:masterfrom
gaborgsomogyi:FLINK-30370
Dec 13, 2022
Merged

[FLINK-30370][runtime][security] Move existing delegation token framework authentication to providers#21499
mbalassi merged 1 commit intoapache:masterfrom
gaborgsomogyi:FLINK-30370

Conversation

@gaborgsomogyi
Copy link
Contributor

What is the purpose of the change

In order to make a generic delegation token manager all authentication specific things must be extracted. This is such a step. In this PR I've moved Kerberos authentication to the provider side.

Brief change log

Moved Kerberos authentication to the provider side.

Verifying this change

Existing unit/integration tests + manually on minikube.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@gaborgsomogyi
Copy link
Contributor Author

cc @mbalassi

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 13, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@mbalassi
Copy link
Contributor

@gaborgsomogyi with the externalization of the HBase connector will the HBaseDelegationTokenProvider continue to live in flink-runtime or should we move it to the hbase connector repo instead longer term (after 1.17)?

cc @ferenc-csaky

@mbalassi mbalassi self-requested a review December 13, 2022 08:42
@gaborgsomogyi
Copy link
Contributor Author

@mbalassi I've mentioned the move possibility to @ferenc-csaky but seems like not yet done. From my point of view we could move it now :)

@mbalassi
Copy link
Contributor

@gaborgsomogyi please take a look at the CI:

Dec 13 08:04:01 [ERROR] Failures: 
Dec 13 08:04:01 [ERROR]   KerberosDelegationTokenManagerITCase.startTokensUpdateShouldScheduleRenewal:184 expected: <true> but was: <false>

@gaborgsomogyi
Copy link
Contributor Author

Just fixed it.

delegationTokenManager.stopTokensUpdate();

assertTrue(retryExceptionThrown.get());
assertEquals(3, startTokensUpdateCallCount.get());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the expectation is still 3 calls:

  • Manual call in line 164
  • Trigger again by exception (retry)
  • Trigger again by normal re-schedule

@ferenc-csaky
Copy link
Contributor

@gaborgsomogyi with the externalization of the HBase connector will the HBaseDelegationTokenProvider continue to live in flink-runtime or should we move it to the hbase connector repo instead longer term (after 1.17)?

cc @ferenc-csaky

I think this should not be involved in the first round of the externalization. We try to have the first externalized connector release being the same we had in 1.16, so the connector code can be removed in 1.17 already. I will just start to port the e2e tests, which has to be done before the connector release, so there are some work to be done on that front.

If there is capacity, PRs can be opened until then, I will be able to help after dealing with the e2e tests.

@gaborgsomogyi
Copy link
Contributor Author

gaborgsomogyi commented Dec 13, 2022

@ferenc-csaky ok, then we can come back to this later in 1.17. At that timepoint we're going to have a relatively stable API.

Copy link
Contributor

@mbalassi mbalassi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mbalassi mbalassi merged commit 4abdf2d into apache:master Dec 13, 2022
@gaborgsomogyi gaborgsomogyi deleted the FLINK-30370 branch September 13, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants