From 97164d9620bc4718f81efbc232ff887967aa9e6f Mon Sep 17 00:00:00 2001 From: Jericho Keyne Date: Wed, 15 Apr 2026 17:18:23 -0400 Subject: [PATCH] OCM-23781 | fix: Adding better logic for setting rosa_region tag --- pkg/aws/client.go | 7 ++-- pkg/aws/cloudformation.go | 58 +++++++++++++++++++++++++------ pkg/ocm/regions.go | 15 ++++---- tests/e2e/test_rosacli_cluster.go | 4 +-- tests/e2e/test_rosacli_region.go | 8 ++--- 5 files changed, 67 insertions(+), 25 deletions(-) diff --git a/pkg/aws/client.go b/pkg/aws/client.go index 2b0d89c774..5812712e64 100644 --- a/pkg/aws/client.go +++ b/pkg/aws/client.go @@ -80,9 +80,10 @@ const ( // Since CloudFormation stacks are region-dependent, we hard-code OCM's default region and // then use it to ensure that the user always gets the stack from the same region. - DefaultRegion = "us-east-1" - Inline = "inline" - Attached = "attached" + DefaultRegion = "us-east-1" + DefaultGovcloudRegion = "us-gov-east-1" + Inline = "inline" + Attached = "attached" LocalZone = "local-zone" WavelengthZone = "wavelength-zone" diff --git a/pkg/aws/cloudformation.go b/pkg/aws/cloudformation.go index 35fc98cbb5..82e2b2e719 100644 --- a/pkg/aws/cloudformation.go +++ b/pkg/aws/cloudformation.go @@ -29,12 +29,13 @@ import ( "github.com/aws/smithy-go" "github.com/openshift/rosa/assets" + "github.com/openshift/rosa/pkg/fedramp" ) func readCloudFormationTemplate(path string) (string, error) { cfTemplate, err := assets.Asset(path) if err != nil { - return "", fmt.Errorf("Unable to read cloudformation template: %s", err) + return "", fmt.Errorf("unable to read cloudformation template: %s", err) } return string(cfTemplate), nil @@ -43,7 +44,17 @@ func readCloudFormationTemplate(path string) (string, error) { // Ensure osdCcsAdmin IAM user is created func (c *awsClient) EnsureOsdCcsAdminUser(stackName string, adminUserName string, awsRegion string) (bool, error) { userExists := true - regionForInit, err := c.GetClusterRegionTagForUser(adminUserName) + var region string + if awsRegion != "" { + region = awsRegion + } else { + if fedramp.Enabled() { + region = DefaultGovcloudRegion + } else { + region = DefaultRegion + } + } + adminRegionTag, err := c.GetClusterRegionTagForUser(adminUserName) if err != nil { //If user doesn't exists proceed normally //If users exists and tag is not present then add the tag @@ -56,8 +67,13 @@ func (c *awsClient) EnsureOsdCcsAdminUser(stackName string, adminUserName string } } if userExists { - if regionForInit == "" { - err = c.TagUserRegion(adminUserName, DefaultRegion) + if adminRegionTag == "" { + err = c.TagUserRegion(adminUserName, region) + if err != nil { + return false, err + } + } else if fedramp.Enabled() && adminRegionTag == DefaultRegion { + err = c.TagUserRegion(adminUserName, region) if err != nil { return false, err } @@ -106,7 +122,7 @@ func (c *awsClient) EnsureOsdCcsAdminUser(stackName string, adminUserName string return false, err } - err = c.TagUserRegion(adminUserName, awsRegion) + err = c.TagUserRegion(adminUserName, region) if err != nil { return false, err } @@ -115,7 +131,15 @@ func (c *awsClient) EnsureOsdCcsAdminUser(stackName string, adminUserName string func (c *awsClient) CreateStack(cfTemplateBody, stackName string) (bool, error) { // Create cloudformation stack - _, err := c.cfClient.CreateStack(context.Background(), buildCreateStackInput(cfTemplateBody, stackName, []cloudformationtypes.Parameter{}, []cloudformationtypes.Tag{})) + _, err := c.cfClient.CreateStack( + context.Background(), + buildCreateStackInput( + cfTemplateBody, + stackName, + []cloudformationtypes.Parameter{}, + []cloudformationtypes.Tag{}, + ), + ) if err != nil { return false, err } @@ -128,7 +152,13 @@ func (c *awsClient) CreateStack(cfTemplateBody, stackName string) (bool, error) return true, nil } -func (c *awsClient) CreateStackWithParamsTags(ctx context.Context, cfTemplateBody, stackName string, stackParams, stackTags map[string]string) (*string, error) { +func (c *awsClient) CreateStackWithParamsTags( + ctx context.Context, + cfTemplateBody, + stackName string, + stackParams, + stackTags map[string]string, +) (*string, error) { // stack tags var cfTags []cloudformationtypes.Tag for k, v := range stackTags { @@ -165,13 +195,16 @@ func (c *awsClient) GetCFStack(ctx context.Context, stackName string) (*cloudfor } if len(output.Stacks) == 0 { - return nil, fmt.Errorf("No CF stacks with name %s found", stackName) + return nil, fmt.Errorf("no CF stacks with name %s found", stackName) } return &output.Stacks[0], nil } -func (c *awsClient) DescribeCFStackResources(ctx context.Context, stackName string) (*[]cloudformationtypes.StackResource, error) { +func (c *awsClient) DescribeCFStackResources( + ctx context.Context, + stackName string, +) (*[]cloudformationtypes.StackResource, error) { output, err := c.cfClient.DescribeStackResources(ctx, &cloudformation.DescribeStackResourcesInput{ StackName: aws.String(stackName), }) @@ -265,7 +298,12 @@ func (c *awsClient) DeleteOsdCcsAdminUser(stackName string) error { } // Build cloudformation create stack input -func buildCreateStackInput(cfTemplateBody, stackName string, cfParams []cloudformationtypes.Parameter, cfTags []cloudformationtypes.Tag) *cloudformation.CreateStackInput { +func buildCreateStackInput( + cfTemplateBody, + stackName string, + cfParams []cloudformationtypes.Parameter, + cfTags []cloudformationtypes.Tag, +) *cloudformation.CreateStackInput { // Special cloudformation capabilities are required to create IAM resources in AWS cfCapabilityIAM := cloudformationtypes.CapabilityCapabilityIam cfCapabilityNamedIAM := cloudformationtypes.CapabilityCapabilityNamedIam diff --git a/pkg/ocm/regions.go b/pkg/ocm/regions.go index 4340657989..27b29c8ee0 100644 --- a/pkg/ocm/regions.go +++ b/pkg/ocm/regions.go @@ -25,6 +25,7 @@ import ( "github.com/openshift/rosa/pkg/arguments" "github.com/openshift/rosa/pkg/aws" + "github.com/openshift/rosa/pkg/fedramp" "github.com/openshift/rosa/pkg/helper" "github.com/openshift/rosa/pkg/logging" ) @@ -96,14 +97,14 @@ func (c *Client) GetRegions(roleARN string, externalID string) (regions []*cmv1. Logger(logger). Build() if err != nil { - return nil, fmt.Errorf("Error creating AWS client: %v", err) + return nil, fmt.Errorf("error creating AWS client: %v", err) } // Get AWS region currentAWSCreds, err := awsClient.GetIAMCredentials() if err != nil { - return nil, fmt.Errorf("Failed to get local AWS credentials: %v", err) + return nil, fmt.Errorf("failed to get local AWS credentials: %v", err) } awsBuilder = awsBuilder. @@ -113,7 +114,7 @@ func (c *Client) GetRegions(roleARN string, externalID string) (regions []*cmv1. awsCredentials, err := awsBuilder.Build() if err != nil { - return nil, fmt.Errorf("Failed to build AWS credentials for user '%s': %v", aws.AdminUserName, err) + return nil, fmt.Errorf("failed to build AWS credentials for user '%s': %v", aws.AdminUserName, err) } collection := c.ocm.ClustersMgmt().V1(). @@ -151,7 +152,7 @@ func (c *Client) GetRegionList(multiAZ bool, roleARN string, regionAZ map[string]bool, err error) { regions, err := c.GetFilteredRegionsByVersion(roleARN, version, awsClient, externalID) if err != nil { - err = fmt.Errorf("Failed to retrieve AWS regions: %s", err) + err = fmt.Errorf("failed to retrieve AWS regions: %s", err) return } @@ -176,7 +177,9 @@ func (c *Client) GetRegionList(multiAZ bool, roleARN string, } func (c *Client) GetDatabaseRegionList() ([]string, error) { - response, err := c.ocm.ClustersMgmt().V1().CloudProviders().CloudProvider("aws").Regions().List().Send() + filter := fmt.Sprintf("govcloud is %t", fedramp.Enabled()) + response, err := + c.ocm.ClustersMgmt().V1().CloudProviders().CloudProvider("aws").Regions().List().Parameter("search", filter).Send() if err != nil { return []string{}, weberr.Errorf("Failed to get regions listing: %v", err) } @@ -202,7 +205,7 @@ func (c *Client) ValidateAwsClientRegion() error { return err } if !helper.Contains(supportedRegions, awsRegionInUserConfig) { - return fmt.Errorf("Unsupported region '%s', available regions: %s", + return fmt.Errorf("unsupported region '%s', available regions: %s", awsRegionInUserConfig, helper.SliceToSortedString(supportedRegions)) } return nil diff --git a/tests/e2e/test_rosacli_cluster.go b/tests/e2e/test_rosacli_cluster.go index 8c23795283..11e411b98b 100644 --- a/tests/e2e/test_rosacli_cluster.go +++ b/tests/e2e/test_rosacli_cluster.go @@ -1366,7 +1366,7 @@ var _ = Describe("Classic cluster creation validation", "2": "Multi AZ cluster requires at least 3 compute nodes", "0": "Multi AZ cluster requires at least 3 compute nodes", "-3": "must be non-negative", - "5": "multi AZ clusters require that the replicas be a multiple of 3", + "5": "Multi AZ clusters require that the replicas be a multiple of 3", } for k, v := range invalidReplicasErrorMapMultiAZ { rosalCommand.ReplaceFlagValue(map[string]string{ @@ -1402,7 +1402,7 @@ var _ = Describe("Classic cluster creation validation", }) stdout, err = rosaClient.Runner.RunCMD(strings.Split(rosalCommand.GetFullCommand(), " ")) Expect(err).NotTo(BeNil()) - Expect(stdout.String()).To(ContainSubstring("Unsupported region")) + Expect(stdout.String()).To(ContainSubstring("unsupported region")) By("Check the validation for invalid billing-account for classic sts cluster") rosalCommand.ReplaceFlagValue(map[string]string{ diff --git a/tests/e2e/test_rosacli_region.go b/tests/e2e/test_rosacli_region.go index d8b6f1889a..eada39f2d5 100644 --- a/tests/e2e/test_rosacli_region.go +++ b/tests/e2e/test_rosacli_region.go @@ -4,8 +4,8 @@ import ( "fmt" "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //nolint:staticcheck + . "github.com/onsi/gomega" //nolint:staticcheck "github.com/openshift/rosa/tests/ci/labels" "github.com/openshift/rosa/tests/utils/exec/rosacli" @@ -19,7 +19,7 @@ var _ = Describe("Region", var ( rosaClient *rosacli.Client ocmResourceService rosacli.OCMResourceService - permissionsBoundaryArn string = "arn:aws:iam::aws:policy/AdministratorAccess" + permissionsBoundaryArn = "arn:aws:iam::aws:policy/AdministratorAccess" ) BeforeEach(func() { @@ -132,6 +132,6 @@ var _ = Describe("Region", availableMachineTypes, output, err := ocmResourceService.ListInstanceTypes( "--region", "xxx", "--role-arn", classicInstallerRoleArn) Expect(err).To(HaveOccurred()) - Expect(output.String()).Should(ContainSubstring("ERR: Unsupported region 'xxx', available regions")) + Expect(output.String()).Should(ContainSubstring("ERR: unsupported region 'xxx', available regions")) }) })