jenkinsci / google-oauth-plugin

This plugin implements the OAuth Credentials interfaces to surface Google Service Account credentials to Jenkins.
https://plugins.jenkins.io/google-oauth-plugin/
Apache License 2.0
37 stars 56 forks source link

Enable creating multiple Google credentials for the same project id #186

Closed Riliane closed 1 year ago

Riliane commented 1 year ago

Enable an ID for GoogleRobotCredentials that is separate from the project ID and migrate the old credentials by backfilling their IDs to the project name, so that the getId() result for them is the same as before. This is needed, for example, when you want to create credentials for 2 different service accounts coming from the same GCP project. I wanted to extend BaseStandardCredentials for this but as their id was NonNull, private and final, I could not rewrite a null ID or account for a null ID in getId() to return the project ID instead.

Testing done

I created a Google Service Account from private key credential with the plugin before the change, then kept that jenkins home. I ran it after my changes and viewed the credentials, saw that the ID populated correctly, tried to create a new credential for the same project, saw it succeed and get a random id. Added an automated test to check the credential migration and another automated test showing we can indeed create two different credentials in the same project. Removed the test to check that the id matches the project id and the test to check that creating a credential in the same project updates the old one, as neither is the now desired behavior.

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [ ] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue
Riliane commented 1 year ago

I'll take care of rewriting the failing tests that expected the old id generating schema when I polish the actual code part of it

amuniz commented 1 year ago

This should be addressing https://github.com/jenkinsci/google-oauth-plugin/issues/12

Riliane commented 1 year ago

Managed to extend from BaseStandardCredentials and get around the finality of the id by swapping an old object with an empty id with a new one on readResolve. I can see that ids keep being regenerated when you update the credential, working on that.

Riliane commented 1 year ago

I suspect the old constructors which are no longer @DataBoundConstructors will now go unused except in tests, I will look into whether I should delete them.

Pldi23 commented 1 year ago

I'm thinking about providing the users a field to specify id manually instead of using auto-generating one (like all other types of credentials)? Is it worth or at current moment it's enough to just unblock multiple service accounts creds per project use case. WDYT?

Riliane commented 1 year ago

can be done as part of this or later in another PR if need arises

Pldi23 commented 1 year ago

@Riliane I realised that I don't see the type of credentials Google Service Account from metadata in the dropdown:

Screenshot 2023-10-03 at 11 06 08

TBH I don't think that it's because of your changes, but do you know is it a existing bug or I miss something in configuration?

Riliane commented 1 year ago

that was there before I made my changes indeed, these were the only credentials that showed up whn I was creating my old format credentials for testing. Unsure why.

jo2y-tock commented 1 year ago

FWIW, this seemed to break our use-case slightly for the kubernetes plugin.

Before, I could (had to) select the project named item in the kubernetes plugin as the credential. After updating with this change, a random uuid was assigned as the id and the kubernetes plugin could no longer find the credential using the project name. Effectively the credential was renamed, but other plugins that referenced it still had the old name in their config.

The credentials UI doesn't allow selecting an specific id when creating this type of credendial. Fortunately for my use-case, we use the config-as-code plugin, and explicitly adding the id field set to the project name does the right thing on reload and the kubernetes plugin is happy again.

Alternatively I probably could have updated the kubernetes config to use the random uuid named credential instead, but I prefer our explicitly configured credentials to have known names.

Riliane commented 1 year ago

@jo2y-tock could you please clarify, do you mean you had existing credentials, that were already in your jenkins home, which got assigned an unpredictable id after you updated - or do you mean you are creating credentials now, after the update, and they are not having the same ids as you would have expected when creating them before? are you creating them on the UI or with CasC?

jo2y-tock commented 1 year ago

@jo2y-tock could you please clarify, do you mean you had existing credentials, that were already in your jenkins home, which got assigned an unpredictable id after you updated - or do you mean you are creating credentials now, after the update, and they are not having the same ids as you would have expected when creating them before? are you creating them on the UI or with CasC?

I had existing credentials that other plugins referenced by $project_name (which I think was the only option before). This change switched the identifier from $project_name to a uuid. So the first option you describe.

I believe I initially created the credential in the UI then used the CasC viewer to see what that keys/values should be and added that to our CasC config. I know our CasC config only had one field set and it was named something other than id. projectName possibly.

FWIW, I've already solved my issue by being more explicit with the CasC config to specify the new values including an id. So this was really just a version transition issue for us and not an ongoing problem.

Riliane commented 1 year ago

and added that to our CasC config. I know our CasC config only had one field set and it was named something other than id.

yes, per our own investigation, while the credentials in the jenkins home if they are not specified with CasC migrate fine, the CasC configs do not migrate on their own - the id is randomized if not specified, and the credential will be recreated with a new random value on restart when CasC is reapplied. it no longer matters that the credential first was created in the UI - you have the CasC config, that now started to cause the credential to be re-generated. this will, indeed, require an update of the user's CasC configs, as I don't currently think there's a way to migrate those configs automatically.

amuniz commented 1 year ago

This is a breaking change (even if it's only happening when using CasC), so it needs to be documented as such in https://github.com/jenkinsci/google-oauth-plugin/releases/tag/google-oauth-plugin-1.1.1-r2. As @jo2y-tock pointed out, the fix is to add an explicit id to the credentials object in CasC yaml.