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 token generation #188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

clementblaise
Copy link
Contributor

@clementblaise clementblaise commented Jul 17, 2024

Description of your changes

Fixes #187

The PR adds ProjectToken to configure tokens and how they should be renewed. It extends the ArgoCD Project client so that the controller can issue tokens. The controller has been updated to add the necessary logic to create and update tokens while saving them as Secret.

I am unsure how to deal with the existing JWTTokens on ProjectRole.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested on a local cluster with a deployment of ArgoCD. Added unit tests

Signed-off-by: Clément Blaise <[email protected]>
@MisterMX
Copy link
Collaborator

I think it is better to implement ProjectToken as a seperate resource instead of including it in Project.

@clementblaise
Copy link
Contributor Author

Hey @MisterMX, is there any reason you think it's a better implementation to have a ProjectToken resource?

I had thought about it but didn't like:

  • Inability to reference a Role because it's defined within the spec of the Project
  • Lead to a breaking change on the Project resources, as the status holds JWTTokensByRole, which will be transferred to ProjectToken
  • Tightly coupled with the Project API, the client needs to GET Project to be aware of the status of tokens which will lead to code duplication

@MisterMX
Copy link
Collaborator

MisterMX commented Aug 2, 2024

I would consider a token its own entity with its own lifecycle. It should be possible to create tokens independently from the project. I would therefor see it as a separate resource.

Can you explain why a breaking change in the project status is necessary?

@clementblaise
Copy link
Contributor Author

@MisterMX Indeed, it is not necessary to break if we choose not to. Tokens are part of a project. Could you explain the benefit of having a dedicated lifecycle?

@MisterMX
Copy link
Collaborator

MisterMX commented Oct 1, 2024

I would argue project tokens do have a separate lifecycle as they can be created and destroyed independently of the project (though they are deleted with the project as well). There might also be the case where the tokens are created separately of the project, i.e. through a separate composition.

All in all I think it makes sense to have a separate resource to have more flexibility at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Project Token Generation
2 participants