Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic support for activeGate auth token #786

Merged
merged 11 commits into from
May 25, 2022

Conversation

luhi-DT
Copy link
Collaborator

@luhi-DT luhi-DT commented May 13, 2022

Description

This change introduces the activeGate auth token support. It basically adds the ability to create an auth token within the operator if the feature flag feature.dynatrace.com/enable-activegate-authtoken: "true" is set on the DynaKube.
This is the first PR of the whole activeGate auth token support feature, as token rotation will be included at a later stage.

How can this be tested?

  1. Create an API Token with the correct permission activeGateTokenManagement.create
  2. Create a Dynakube that also creates an activeGate and add feature.dynatrace.com/enable-activegate-authtoken: "true" to it
  3. check if the activeGate gets the correct secret mounted and check the logs of the activeGate if it uses the created auth Token

Checklist

  • Unit tests have been updated/added
  • PR is labeled accordingly

@luhi-DT luhi-DT added the core Changes to core functionality of the Operator label May 13, 2022
@luhi-DT luhi-DT requested a review from a team as a code owner May 13, 2022 07:52
Copy link
Contributor

@0sewa0 0sewa0 left a comment

Choose a reason for hiding this comment

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

My reviews aside, I reeally don't like the idea of having both a status field based on the scopes and a feature-flag to turn the thing on-off.
This means we have 2 different knobs that kinda do the same thing. (and rather confusing)
The user can opt out 2 different ways, by not having the scopes for their token OR adding the feature-flag.
(I guess) most users still have to opt-in, because they need to update the tokens.

I know the reason for having this is so that the Deployment Page just creates a token with the correct scopes by default and the authToken will be used unless explicitly turned off.
Couldn't we have just 1 feature flag to turn the thing on and the Deployment page could add that annotation to the dynakube by default instead ?

In this case, if turned on and scopes are not correct, we complain (like with the automatic kubernetes monitoring registration).
(And in the future when having this new scope is the norm, we could flip the default value of the feature-flag to true, doing so the Deployment page is safe as even if they still set the feature-flag, because it will only be redundant)

src/api/v1beta1/dynakube_status.go Outdated Show resolved Hide resolved
src/api/v1beta1/properties.go Show resolved Hide resolved
config/deploy/kubernetes/kubernetes-all.yaml Outdated Show resolved Hide resolved
src/api/v1beta1/properties.go Show resolved Hide resolved
src/controllers/dynakube/dtclient_reconciler.go Outdated Show resolved Hide resolved
src/controllers/certificates/certificate_secret.go Outdated Show resolved Hide resolved
src/kubeobjects/secret.go Outdated Show resolved Hide resolved
src/controllers/dynakube/dtclient_reconciler.go Outdated Show resolved Hide resolved
src/dtclient/activegate_auth_token.go Show resolved Hide resolved
src/dtclient/activegate_auth_token.go Outdated Show resolved Hide resolved
src/dtclient/activegate_auth_token.go Show resolved Hide resolved
src/dtclient/dynatrace_client.go Show resolved Hide resolved
@0sewa0 0sewa0 dismissed mjgrzybek’s stale review May 25, 2022 09:05

changes were made

@luhi-DT luhi-DT merged commit 75b0f45 into master May 25, 2022
@luhi-DT luhi-DT deleted the feature/add-ag-auth-token-support branch May 25, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core functionality of the Operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants