Draft implementation for Issue #7 with aws secret manager #16
Conversation
|
Thanks! Will take a look. |
|
@zhill I am not sure if I put the change in the right place, and feel free let me know your thoughts. |
There was a problem hiding this comment.
Good start! Thanks! I'd like to setup a structure that will support more extensible auth managers. If you'd like to implement a factory interface to return something like a 'CredentialProvider' object that returns valid credentials based on the configuration that would be ideal. Part of that would be putting the aws code in its own package, and imported by the factory so that the client just imports the factory and gets credentials from it. That will allow simpler testing of clients and mocks for the client, as well as extensions for Azure Key Vault or other similar services moving forward.
pkg/adapter/anchore/client/client.go
Outdated
| timeout := time.Duration(clientConfiguration.TimeoutSeconds) * time.Second | ||
| return gorequest.New().TLSClientConfig(&tls.Config{InsecureSkipVerify: clientConfiguration.TLSVerify}).SetBasicAuth(clientConfiguration.Username, clientConfiguration.Password).Timeout(timeout) | ||
| } | ||
|
|
||
| // Reload the anchore user name and password if AWS Secret Manager is used | ||
| func reloadCredential(clientConfiguration *ClientConfig) { |
There was a problem hiding this comment.
Rather than a reload, I think a credential factory model makes more sense. It would take a clientConfiguration and return a set of credentials. The default factory would return the credential in the config, but the AWS Credential Factory would return the credentials from the SecretManager. I think that would make testing the individual parts a little easier and allow a clean extension point later to use other credential managers or service accounts.
There was a problem hiding this comment.
@zhill I am totally fine with a factory pattern, can you draft the codes structure and I will push an aws implementation.
|
@zhill I applied the factory pattern, but as I am new for golang, could you pls help review it again and share me your comments? Merry x'mas and happy new year! ye |
Thanks! I'll give it a review after the holiday. Thanks very much for the contribution and collaboration. Happy holidays to you as well! |
|
@zhill happy new year, can you make a quick review on it? |
|
@cafeliker yes, I'll review it tonight |
|
|
||
| type DefaultCredenitalLoader struct{} | ||
|
|
||
| func (c *DefaultCredenitalLoader) LoadFromCredentialStore(passwordConfig string) string { |
There was a problem hiding this comment.
I'd like to make the secret/cred configuration more explicit rather than an encoded value in the password, but I think we can start with this as a draft and follow-up with another commit to change that behavior before a release is pushed
There was a problem hiding this comment.
Sure, we can talk after Feb 3 as I will be in Chinese new year holiday from next week. I'd like to hear more ideas from you ;-)
There was a problem hiding this comment.
Have a great new year holiday! We'll sync up when you return.
|
I can either squash-merge it or you can rebase/squash it down to one commit if you'd like to set the commit message that will show in the change-log. |
|
@zhill it will be easier if you do squash-merge through the github UI, pls do it, thanks. |
|
I should have some time tomorrow to get to this. I've not forgotten, just short on cycles recently. |
Draft implementation on #7