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

feat(permissions): Add permissions support for accounts in halyard. #954

Merged
merged 1 commit into from
May 31, 2018

Conversation

stewchen
Copy link
Contributor

This change also removes the deprecated --requiredGroupMembership flags,
as they are no longer functional.

@stewchen stewchen requested review from lwander and ttomsu May 29, 2018 21:30
Copy link
Member

@lwander lwander left a comment

Choose a reason for hiding this comment

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

Please keep around the old flags for now

@@ -42,10 +43,17 @@

@Parameter(
variableArity = true,
names = "--required-group-membership",
description = AccountCommandProperties.REQUIRED_GROUP_MEMBERSHIP_DESCRIPTION
names = "--read-permissions",
Copy link
Member

Choose a reason for hiding this comment

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

These flags may no longer be functional in the latest spinnaker release, but still work in older ones. I would not remove them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

@lwander
Is there a deprecation process by which these flags should be removed at some time in the future?

Copy link
Member

Choose a reason for hiding this comment

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

There is an annotation for flags that don't work until a certain release -- we need to add/extend that annotation for flags that no longer work after a certain release. That can be done in a separate PR.

@stewchen
Copy link
Contributor Author

Reverted the deletions

@@ -21,4 +21,8 @@
public static final String REQUIRED_GROUP_MEMBERSHIP_DESCRIPTION = "A user must be a member of at least one specified group in order to make changes to this account's cloud resources.";

public static final String PROVIDER_VERSION_DESCRIPTION = "Some providers support multiple versions/release tracks. This allows you to pick the version of the provider (not the resources it manages) to run within Spinnaker.";

public static final String READ_PERMISSION_DESCRIPTION = "A user must have matching roles of all of these read permissions in order to view this account's cloud resources.";
Copy link
Member

Choose a reason for hiding this comment

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

A user must have _at least one_ of these roles in order to be granted the READ` permission. Same with the write description

@stewchen stewchen merged commit 1eb9897 into spinnaker:master May 31, 2018
@stewchen stewchen deleted the permissions branch May 31, 2018 18:31
@stewchen
Copy link
Contributor Author

Docs update pending the release: spinnaker/spinnaker.github.io#813

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.

4 participants