-
Notifications
You must be signed in to change notification settings - Fork 3
VAULT-31184: Add support for token auth in enos-flight-control download
#31
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
Conversation
…ed test and bumped version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great. Unfortunately it's been > 3 months since our last commit so we'll have to resolve some CI issues in addition to your change.
Most, if not all, of the static analysis failures are due to new linters added to golangci-lint since the last time things were run. I think it's safe to disable those in the golangci-lint.yml (unless you want to "fix" the issues). You'll want to install golangci-lint to run the linters locally. First run make lint-fix to fix anything that can be automatically fixed, then make lint to see what is failing.
The acceptance test host faliure is likely because our target host was an Ubuntu version that no longer exists. We'll probably want to update the terraform module to use Ubuntu 24.04. It should be pretty easy to update the failing data source to find that. There's also plenty, but more complicated, prior art in the ec2_info module in vault.
| a.flags.StringVar(&a.sha256, "sha256", "", "if given, verifies that the downloaded file matches the given SHA 256 sum") | ||
| a.flags.StringVar(&a.authUser, "auth-user", "", "if given, sets the basic auth username when making the HTTP request") | ||
| a.flags.StringVar(&a.authPassword, "auth-password", "", "if given, sets the basic auth password when making the HTTP request") | ||
| a.flags.StringVar(&a.authToken, "auth-token", "", "if given, sets the auth token when making the HTTP request") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nice to note in both the auth password/user and auth token help that only one or the other ought to be used.
| Funcs(transportRenderFunc). | ||
| Parse(`resource "enos_bundle_install" "{{.ID.Value}}" { | ||
| destination = "{{ .Destination.Value }}" | ||
| destination = "{{ .Destination.Value }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't matter much but the config here will render huge with all those tabs if the test fails. Generally we try and use two spaces in HCL blocks in tests.
enos-flight-control download
3e44eb3 to
b991149
Compare
How to read this pull request
These changes will enable using an identity token instead of a username with API key in Artifactory requests. To maintain backward compatibilIty, username is still an option, but is not required. This PR builds on changes introduced in Ryan's PR.
I ran the following scenario locally with token as well as username/ API key in order to confirm that both auth options work:
agent arch:amd64 artifact_type:bundle artifact_source:artifactory config_mode:file distro:ubuntu edition:ce seal:shamir backend:raft consul_edition:ce ip_version:6 consul_version:1.21.1Not directly related, but I also updated our Go modules.
Checklist