Skip to content

Commit

Permalink
Merge pull request #1029 from gdbranco/fix/consider-current-version-i…
Browse files Browse the repository at this point in the history
…ncompatible

[SDA-7898] Add force param to forcefully create policies
  • Loading branch information
openshift-merge-robot authored Jan 25, 2023
2 parents 7a533f5 + 930a9d5 commit 2ee6004
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 25 deletions.
26 changes: 23 additions & 3 deletions cmd/create/accountroles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var args struct {
version string
channelGroup string
managed bool
forcePolicyCreation bool
}

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -108,6 +109,14 @@ func init() {
)
flags.MarkHidden("managed")

flags.BoolVarP(
&args.forcePolicyCreation,
"force-policy-creation",
"f",
false,
"Forces creation of policies skipping compatibility check",
)

aws.AddModeFlag(Cmd)

confirm.AddFlag(flags)
Expand Down Expand Up @@ -281,7 +290,13 @@ func run(cmd *cobra.Command, argv []string) {
os.Exit(1)
}
}
policies, err := r.OCMClient.GetPolicies("")

if args.forcePolicyCreation && mode != aws.ModeAuto {
r.Reporter.Warnf("Forcing creation of policies only works in auto mode")
os.Exit(1)
}

policies, err := r.OCMClient.GetPolicies("AccountRole")
if err != nil {
r.Reporter.Errorf("Expected a valid role creation mode: %s", err)
os.Exit(1)
Expand Down Expand Up @@ -447,8 +462,13 @@ func createRoles(r *rosa.Runtime, prefix, permissionsBoundary, accountID, env st
policyPermissionDetail := aws.GetPolicyDetails(policies, filename)

r.Reporter.Debugf("Creating permission policy '%s'", policyARN)
policyARN, err = r.AWSClient.EnsurePolicy(policyARN, policyPermissionDetail,
defaultPolicyVersion, tagsList, path)
if args.forcePolicyCreation {
policyARN, err = r.AWSClient.ForceEnsurePolicy(policyARN, policyPermissionDetail,
defaultPolicyVersion, tagsList, path)
} else {
policyARN, err = r.AWSClient.EnsurePolicy(policyARN, policyPermissionDetail,
defaultPolicyVersion, tagsList, path)
}
if err != nil {
return err
}
Expand Down
53 changes: 37 additions & 16 deletions cmd/create/operatorroles/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
var args struct {
prefix string
permissionsBoundary string
forcePolicyCreation bool
}

var Cmd = &cobra.Command{
Expand Down Expand Up @@ -73,6 +74,14 @@ func init() {
"The ARN of the policy that is used to set the permissions boundary for the operator roles.",
)

flags.BoolVarP(
&args.forcePolicyCreation,
"force-policy-creation",
"f",
false,
"Forces creation of policies skipping compatibility check",
)

aws.AddModeFlag(Cmd)
confirm.AddFlag(flags)
interactive.AddFlag(flags)
Expand Down Expand Up @@ -126,15 +135,15 @@ func run(cmd *cobra.Command, argv []string) {
}

if len(missingRoles) == 0 &&
cluster.State() != cmv1.ClusterStateWaiting && cluster.State() != cmv1.ClusterStatePending {
cluster.State() != cmv1.ClusterStateWaiting && cluster.State() != cmv1.ClusterStatePending &&
!args.forcePolicyCreation {
r.Reporter.Infof("Cluster '%s' is %s and does not need additional configuration.",
clusterKey, cluster.State())
os.Exit(0)
}

prefix, err := aws.GetPrefixFromInstallerAccountRole(cluster)
if err != nil {
r.Reporter.Errorf("Failed to find prefix from %s account role", aws.InstallerAccountRole)
if args.forcePolicyCreation && mode != aws.ModeAuto {
r.Reporter.Warnf("Forcing creation of policies only works in auto mode")
os.Exit(1)
}

Expand Down Expand Up @@ -181,14 +190,16 @@ func run(cmd *cobra.Command, argv []string) {
r.Reporter.Errorf("Expected parsing role account role '%s': %v", cluster.AWS().STS().RoleARN(), err)
os.Exit(1)
}

path, err := getPathFromInstallerRole(cluster)
if err != nil {
r.Reporter.Errorf("Expected a valid path for '%s': %v", cluster.AWS().STS().RoleARN(), err)
os.Exit(1)
}
if path != "" && !output.HasFlag() && r.Reporter.IsTerminal() {
r.Reporter.Infof("ARN path '%s' detected. This ARN path will be used for subsequent"+
" created operator roles and policies, for the account roles with prefix '%s'", path, prefix)
r.Reporter.Infof("ARN path '%s' detected in installer role '%s'. "+
"This ARN path will be used for subsequent created operator roles and policies.",
path, cluster.AWS().STS().RoleARN())
}
accountRoleVersion, err := r.AWSClient.GetAccountRoleVersion(roleName)
if err != nil {
Expand Down Expand Up @@ -326,16 +337,26 @@ func createRoles(r *rosa.Runtime,
operator.Name(), path)
policyDetails := aws.GetPolicyDetails(policies, filename)

policyARN, err = r.AWSClient.EnsurePolicy(policyARN, policyDetails,
defaultVersion, map[string]string{
tags.OpenShiftVersion: accountRoleVersion,
tags.RolePrefix: prefix,
tags.RedHatManaged: helper.True,
tags.OperatorNamespace: operator.Namespace(),
tags.OperatorName: operator.Name(),
}, path)
if err != nil {
return err
operatorPolicyTags := map[string]string{
tags.OpenShiftVersion: accountRoleVersion,
tags.RolePrefix: prefix,
tags.RedHatManaged: helper.True,
tags.OperatorNamespace: operator.Namespace(),
tags.OperatorName: operator.Name(),
}

if args.forcePolicyCreation {
policyARN, err = r.AWSClient.ForceEnsurePolicy(policyARN, policyDetails,
defaultVersion, operatorPolicyTags, path)
if err != nil {
return err
}
} else {
policyARN, err = r.AWSClient.EnsurePolicy(policyARN, policyDetails,
defaultVersion, operatorPolicyTags, path)
if err != nil {
return err
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/aws/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ type Client interface {
version string, tagList map[string]string, path string, managedPolicies bool) (string, error)
ValidateRoleNameAvailable(name string) (err error)
PutRolePolicy(roleName string, policyName string, policy string) error
ForceEnsurePolicy(policyArn string, document string, version string, tagList map[string]string,
path string) (string, error)
EnsurePolicy(policyArn string, document string, version string, tagList map[string]string,
path string) (string, error)
AttachRolePolicy(roleName string, policyARN string) error
Expand Down
23 changes: 17 additions & 6 deletions pkg/aws/policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,18 @@ func (c *awsClient) PutRolePolicy(roleName string, policyName string, policy str
return nil
}

func (c *awsClient) ForceEnsurePolicy(policyArn string, document string,
version string, tagList map[string]string, path string) (string, error) {
return c.ensurePolicyHelper(policyArn, document, version, tagList, path, true)
}

func (c *awsClient) EnsurePolicy(policyArn string, document string,
version string, tagList map[string]string, path string) (string, error) {
return c.ensurePolicyHelper(policyArn, document, version, tagList, path, false)
}

func (c *awsClient) ensurePolicyHelper(policyArn string, document string,
version string, tagList map[string]string, path string, force bool) (string, error) {
output, err := c.IsPolicyExists(policyArn)
if err != nil {
var policyArnLocal string
Expand All @@ -284,9 +294,12 @@ func (c *awsClient) EnsurePolicy(policyArn string, document string,

policyArn = aws.StringValue(output.Policy.Arn)

isCompatible, err := c.IsPolicyCompatible(policyArn, version)
if err != nil {
return policyArn, err
isCompatible := false
if !force {
isCompatible, err = c.IsPolicyCompatible(policyArn, version)
if err != nil {
return policyArn, err
}
}

if !isCompatible {
Expand Down Expand Up @@ -384,9 +397,7 @@ func (c *awsClient) HasCompatibleVersionTags(iamTags []*iam.Tag, version string)
if err != nil {
return false, err
}
// Current version equals to wanted is not necessarily compatible
// as actions can be altered to accommodate more permissions
return currentVersion.GreaterThan(wantedVersion), nil
return currentVersion.GreaterThanOrEqual(wantedVersion), nil
}
}
return false, nil
Expand Down

0 comments on commit 2ee6004

Please sign in to comment.