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

Initial development #1

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Initial development #1

wants to merge 4 commits into from

Conversation

chriskilding
Copy link
Owner

@chriskilding chriskilding commented Feb 13, 2023

Create the first version of the plugin

To do

  • Make folder-scoped secret fetching work
  • Docs
  • Tests
  • Bug fixes
  • Code cleanup
  • Refactor upstream plugin as needed to support this

@chriskilding
Copy link
Owner Author

Looping in @edwardprzeniczny

@chriskilding
Copy link
Owner Author

In its current state the folder-scoped provider is called, however due to the code paths that are followed the ItemGroup that's passed in is always Jenkins.get() i.e. the root Hudson object. This is wrong because we want the relevant AbstractFolder (which subclasses ItemGroup) to be passed in instead.

@chriskilding
Copy link
Owner Author

@edwardprzeniczny I've now got something early stage and experimental which should let you configure the plugin at a folder level, and let jobs in that folder fetch folder-scoped credentials.

Would you be able to test it* and let me know if it generally does what you're looking for?

If you're comfortable building the plugin from source then please do that, if not then I can compile a .hpi plugin file for you.

(Note that there are some features missing at the moment: I haven't implemented credential caching, or hierarchical config merging. It also has a hard dependency on the main Secrets Manager plugin at the moment, so that will attempt a connection to Secrets Manager with its own settings, irrespective of your folder-level settings.)


(*) Please only use this on a test Jenkins, don't let it anywhere near production for now

@chriskilding
Copy link
Owner Author

CC @NoamGoren @tuxy85 @alandevine @maqzee-git @sun-mir @Sickafant

@edwardprzeniczny
Copy link

@chriskilding Apologies for the late response. Yes I am willing and able to test this in test environment. It is also cloudbees jenkins so i can help out in validating you cannot

Currently attempting to build from source - will have to see how long that dependency resolution takes, lol

@edwardprzeniczny
Copy link

@chriskilding So I built it and deployed it (and installed the aws-secrets-manager-credentials-provider) onto a clean instance - no issues

Configured folder level config with assumable role + session name (might be a good idea to note this is a requirement) and it loaded the credential (just one so far) in to the folder configuration

Configured global configuration to use the default (controller) role and it loaded credentials at global level

Configured a second folder (no settings for secrets manager credential) and ended up with another version of the secret thats configured at a global level. It might be a good idea to have an enable/disable?

One quirk I have found so far is in the Jenkins -> Manage -> Manage Credentials I see a AWS Secrets Manager Folder in the scope
image

On the whole, however, this is excellent work

@chriskilding
Copy link
Owner Author

Thank you for testing!

Re the duplicate credential you saw which was in both the global store and a folder store - was this credential for the same underlying secret (in the same AWS account)?

Or were the global store and the folder store pointed at different AWS accounts, in which case there just happen to be 2 secrets with the same name in your different accounts? (This often occurs when you have an AWS account per environment and you use templating e.g. with Terraform to make them look identical)

@edwardprzeniczny
Copy link

@chriskilding Yes, It was the same underlying secret from the same account (the one where the controller is launched). The default is to use the controllers profile and so every folder is pulling the secrets from the controllers aws account, unless its configured to look elsewhere. Not every folder will require credentials and so I think the default should be to be disabled.

@chriskilding
Copy link
Owner Author

Hmm... the folder-scoped CredentialsProvider has a null guard on the folder-level configuration. This is supposed to ensure that if there's no folder-level configuration present on the specified folder, the folder-scoped CredentialsProvider will act like it does not exist, and return an empty list of credentials.

This should mean that these folders will only see the global scoped credentials, and there won't be a duplicate.

If, however, you have folder-level configuration present, and it points at the same AWS account as your global config, then both providers will attempt to resolve it. In this scenario you would see duplicate credentials.

Does this match up with what you've seen?

@edwardprzeniczny
Copy link

So firstly, I am creating folders in the UI - we haven't yet reached a setup where everything is casc-able

If I create a folder, I end up at the folder configuration page. If I return to another page then the secrets manager isnt enabled on this folder. I can see that by going to these new folder credentials and can see everything is as it should be.

As soon as I click apply/save, aws secrets are enabled on the folder due to the default being being auto-selected in the form.

Essentially, no setting can be changed on the folder through the UI without enabling the aws secrets manager credentials on the folder

@chriskilding
Copy link
Owner Author

Ahhh right, I see now. In the unit/integration tests everything was driven from CasC / JobDSL, so I hadn't checked the Web UI approach.

Yes, we'll definitely need a way to stop this implicit configuration of the folder-scoped credential provider, on folders where it's not wanted.

@chriskilding
Copy link
Owner Author

Hi again, I've pushed a change which puts the folder-scoped credentials provider's configuration behind an 'Enabled` checkbox. This addresses the problem you found, allowing you to set folder configuration in the Web UI, while letting you control whether the folder-scoped credentials provider is enabled on that folder.

Give it a spin and let me know if it works for you :)

@chriskilding
Copy link
Owner Author

@edwardprzeniczny would you be able to test the updated version of the plugin, and let me know if it fixes the problem you encountered?

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.

None yet

2 participants