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

deployment-service: reconcile this code with automate-cli usage #951

Conversation

srenatus
Copy link
Contributor

For reasons that might have disappeared from everyone's memory, the d-s
code was also what drove chef-automate admin-token. So, I've removed
any "diagnostics"-related wording from the V2 policy data.

Note that chef-automate admin-token first checks if the system uses
v2, so that code path we've recently added for diagnostics should NOT be
executed in that scenario. It still felt cleaner to remove the
"diagnostics" words.

Furthermore, this changes GenerateAdminToken so that in the
diagnostics+iam-v2 use case, it can be run twice: once before the
upgrade, once after; without triggering any policy conflicts.

Before this commit, the second run would have not failed completely, but
the already existing policy would not have referenced the proper token
ID. Now, the v2 code is analogous to the v1 code: one policy per token.
While we could do something more clever, this is enough for the limited
use case I'm after.

If we did do something more clever, we might as well make chef-automate admin-token not freak out if A2 is running on IAMv2.

For reasons that might have disappeared from everyone's memory, the d-s
code was also what drove `chef-automate admin-token`. So, I've removed
any "diagnostics"-related wording from the V2 policy data.

Note that `chef-automate admin-token` first checks if the system uses
v2, so that code path we've recently added for diagnostics should NOT be
executed in that scenario. It still felt cleaner to remove the
"diagnostics" words.

Furthermore, this changes `GenerateAdminToken` so that in the
diagnostics+iam-v2 use case, it can be run twice: once before the
upgrade, once after; without triggering any policy conflicts.

Before this commit, the second run would have not failed completely, but
the already existing policy would not have referenced the proper token
ID. Now, the v2 code is analogous to the v1 code: one policy per token.
While we could do something more clever, this is enough for the limited
use case I'm after.

If we did do something more clever, we might as well make `chef-automate
admin-token` not freak out if A2 is running on IAMv2.

Signed-off-by: Stephan Renatus <[email protected]>
@srenatus srenatus added the auth-team anything that needs to be on the auth team board label Jul 18, 2019
@srenatus srenatus self-assigned this Jul 18, 2019
Copy link
Contributor

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have high hopes for this one!

@srenatus srenatus merged commit ce0b377 into master Jul 18, 2019
@chef-ci chef-ci deleted the sr/diagnostics/admin-token-with-iam-v2-run-twice-second-attempt branch July 18, 2019 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants