From 8cee6045fe3707cd3a8d892d9dae317c75109e8d Mon Sep 17 00:00:00 2001 From: pvasanth Date: Tue, 1 Mar 2022 11:48:50 -0500 Subject: [PATCH] fix upgrade issue --- cmd/upgrade/accountroles/cmd.go | 19 +++++-------------- cmd/upgrade/cluster/cmd.go | 9 +-------- cmd/upgrade/operatorroles/cmd.go | 2 -- pkg/aws/policies.go | 12 +++++------- pkg/sts/helper.go | 31 ------------------------------- 5 files changed, 11 insertions(+), 62 deletions(-) delete mode 100644 pkg/sts/helper.go diff --git a/cmd/upgrade/accountroles/cmd.go b/cmd/upgrade/accountroles/cmd.go index 739833cb6..8093075e5 100644 --- a/cmd/upgrade/accountroles/cmd.go +++ b/cmd/upgrade/accountroles/cmd.go @@ -64,15 +64,6 @@ func init() { ) Cmd.MarkFlagRequired("prefix") - flags.StringVarP( - &args.clusterID, - "clusterID", - "c", - "", - "", - ) - flags.MarkHidden("clusterID") - confirm.AddFlag(flags) interactive.AddFlag(flags) } @@ -83,13 +74,15 @@ func run(cmd *cobra.Command, argv []string) error { isInvokedFromClusterUpgrade := false skipInteractive := false - if len(argv) == 2 && !cmd.Flag("prefix").Changed { + if len(argv) >= 2 && !cmd.Flag("prefix").Changed { args.prefix = argv[0] aws.SetModeKey(argv[1]) - if argv[1] != "" { skipInteractive = true } + if len(argv) > 2 && argv[2] != "" { + args.clusterID = argv[2] + } isInvokedFromClusterUpgrade = true } args.isInvokedFromClusterUpgrade = isInvokedFromClusterUpgrade @@ -98,9 +91,7 @@ func run(cmd *cobra.Command, argv []string) error { reporter.Errorf("%s", err) os.Exit(1) } - prefix := args.prefix - // Create the AWS client: awsClient, err := aws.NewClient(). Logger(logger). @@ -246,7 +237,7 @@ func run(cmd *cobra.Command, argv []string) error { func upgradeAccountRolePolicies(reporter *rprtr.Object, awsClient aws.Client, prefix string, accountID string) error { for file, role := range aws.AccountRoles { name := aws.GetRoleName(prefix, role.Name) - if !confirm.Prompt(true, "Upgrade the '%s' role polices to version %s?", name, + if !confirm.Prompt(true, "Upgrade the '%s' role policy to version %s?", name, aws.DefaultPolicyVersion) { if args.isInvokedFromClusterUpgrade { return reporter.Errorf("Account roles need to be upgraded to proceed" + diff --git a/cmd/upgrade/cluster/cmd.go b/cmd/upgrade/cluster/cmd.go index bd8010468..23f5cbbe4 100644 --- a/cmd/upgrade/cluster/cmd.go +++ b/cmd/upgrade/cluster/cmd.go @@ -18,9 +18,6 @@ package cluster import ( "fmt" - "github.com/openshift/rosa/cmd/upgrade/operatorroles" - "github.com/openshift/rosa/pkg/sts" - "os" "strconv" "strings" @@ -242,7 +239,7 @@ func run(cmd *cobra.Command, _ []string) { reporter.Errorf("Could not get role prefix for cluster '%s' : %v", clusterKey, err) os.Exit(1) } - err = accountroles.Cmd.RunE(accountroles.Cmd, []string{prefix, mode}) + err = accountroles.Cmd.RunE(accountroles.Cmd, []string{prefix, mode, cluster.ID()}) if err != nil { accountRoleStr := fmt.Sprintf("rosa upgrade account-roles --prefix %s", prefix) operatorRoleStr := fmt.Sprintf("rosa upgrade operator-roles -c %s", clusterKey) @@ -253,10 +250,6 @@ func run(cmd *cobra.Command, _ []string) { "\t%s\n", version, accountRoleStr, operatorRoleStr) os.Exit(0) } - //Check if the new roles are needed and if so call the update operator role - if sts.IsNewOperatorAdded(version){ - err = accountroles.Cmd.RunE(operatorroles.Cmd, []string{prefix, mode}) - } reporter.Infof("Account and operator roles for cluster '%s' are compatible with upgrade", clusterKey) } diff --git a/cmd/upgrade/operatorroles/cmd.go b/cmd/upgrade/operatorroles/cmd.go index 6d509645b..031a7bc6a 100644 --- a/cmd/upgrade/operatorroles/cmd.go +++ b/cmd/upgrade/operatorroles/cmd.go @@ -179,8 +179,6 @@ func run(cmd *cobra.Command, argv []string) { reporter.Errorf("Error upgrading the role polices: %s", err) os.Exit(1) } - //create the new role and call the ocm to add it - case aws.ModeManual: err = aws.GenerateOperatorPolicyFiles(reporter) if err != nil { diff --git a/pkg/aws/policies.go b/pkg/aws/policies.go index 7603975fc..9e5081bc0 100644 --- a/pkg/aws/policies.go +++ b/pkg/aws/policies.go @@ -42,7 +42,6 @@ type Operator struct { Name string Namespace string ServiceAccountNames []string - Version string } var CredentialRequests map[string]Operator = map[string]Operator{ @@ -82,7 +81,6 @@ var CredentialRequests map[string]Operator = map[string]Operator{ "aws-ebs-csi-driver-operator", "aws-ebs-csi-driver-controller-sa", }, - Version: "4.10", }, } @@ -1455,9 +1453,6 @@ func (c *awsClient) IsUpgradedNeededForOperatorRolePolicies(cluster *cmv1.Cluste func (c *awsClient) IsUpgradedNeededForOperatorRolePoliciesUsingPrefix(prefix string, accountID string, version string) (bool, error) { for _, operator := range CredentialRequests { - if operator.Version == version{ - return true,nil - } policyARN := GetOperatorPolicyARN(accountID, prefix, operator.Namespace, operator.Name) isCompatible, err := c.isRolePoliciesCompatibleForUpgrade(policyARN, version) if err != nil { @@ -1477,6 +1472,9 @@ func (c *awsClient) validateRoleUpgradeVersionCompatibility(roleName string, return false, err } for _, attachedPolicy := range attachedPolicies { + if attachedPolicy.PolicyArn == "" { + continue + } isCompatible, err := c.isRolePoliciesCompatibleForUpgrade(attachedPolicy.PolicyArn, version) if err != nil { return false, errors.Errorf("Failed to validate role polices : %v", err) @@ -1489,13 +1487,13 @@ func (c *awsClient) validateRoleUpgradeVersionCompatibility(roleName string, } func (c *awsClient) isRolePoliciesCompatibleForUpgrade(policyARN string, version string) (bool, error) { - policyTagOutput, err := c.iamClient.GetPolicy(&iam.GetPolicyInput{ + policyTagOutput, err := c.iamClient.ListPolicyTags(&iam.ListPolicyTagsInput{ PolicyArn: aws.String(policyARN), }) if err != nil { return false, err } - return c.hasCompatibleMajorMinorVersionTags(policyTagOutput.Policy.Tags, version) + return c.hasCompatibleMajorMinorVersionTags(policyTagOutput.Tags, version) } func (c *awsClient) GetAccountRoleVersion(roleName string) (string, error) { diff --git a/pkg/sts/helper.go b/pkg/sts/helper.go deleted file mode 100644 index f34dec3e1..000000000 --- a/pkg/sts/helper.go +++ /dev/null @@ -1,31 +0,0 @@ -package sts - -import ( - cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" - "github.com/openshift/rosa/pkg/aws" -) - -func GetNewOperatorAdded(cluster *cmv1.Cluster) map[string]aws.Operator { - newCredRequest := make(map[string]aws.Operator) - for credRequest,operator := range aws.CredentialRequests{ - exists:=false - for _, role := range cluster.AWS().STS().OperatorIAMRoles() { - if role.Namespace() == operator.Namespace && role.Name() == operator.Name { - exists = true - } - } - if !exists{ - newCredRequest[credRequest]= operator - } - } - return newCredRequest -} - -func IsNewOperatorAdded(version string) bool { - for _, operator := range aws.CredentialRequests { - if operator.Version == version { - return true - } - } - return false -}