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

OCM-12828 | fix: Do not print create policy commands more than once op roles #2660

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 36 additions & 28 deletions cmd/create/operatorroles/by_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,8 @@ func buildCommandsFromPrefix(r *rosa.Runtime, env string,
}

isSharedVpc := sharedVpcRoleArn != ""
var policyDetails = make(map[string]roles.ManualSharedVpcPolicyDetails)

commands := []string{}

for credrequest, operator := range credRequests {
Expand Down Expand Up @@ -529,52 +531,58 @@ func buildCommandsFromPrefix(r *rosa.Runtime, env string,

// Precreate HCP shared VPC policies for less memory usage + time to execute
// Shared VPC role arn (route53)
var policyDetails = make(map[string]roles.ManualSharedVpcPolicyDetails)
exists, createPolicyCommand, policyName, err := roles.GetHcpSharedVpcPolicyDetails(r, sharedVpcRoleArn)
if err != nil {
return "", err
}
policyDetails[aws.IngressOperatorCloudCredentialsRoleType] = roles.ManualSharedVpcPolicyDetails{
Command: createPolicyCommand,
Name: policyName,
AlreadyExists: exists,
if _, ok := policyDetails[aws.IngressOperatorCloudCredentialsRoleType]; !ok {
exists, createPolicyCommand, policyName, err := roles.GetHcpSharedVpcPolicyDetails(r, sharedVpcRoleArn)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be modified to use a common function since L534 -543 and L546 - 557 are pretty much the same

if err != nil {
return "", err
}
policyDetails[aws.IngressOperatorCloudCredentialsRoleType] = roles.ManualSharedVpcPolicyDetails{
Command: createPolicyCommand,
Name: policyName,
AlreadyExists: exists,
}
}
// VPC endpoint role arn
exists, createPolicyCommand, policyName, err = roles.GetHcpSharedVpcPolicyDetails(r, vpcEndpointRoleArn)
if err != nil {
return "", err
}
if _, ok := policyDetails[aws.ControlPlaneCloudCredentialsRoleType]; !ok {

policyDetails[aws.ControlPlaneCloudCredentialsRoleType] = roles.ManualSharedVpcPolicyDetails{
Command: createPolicyCommand,
Name: policyName,
AlreadyExists: exists,
exists, createPolicyCommand, policyName, err := roles.GetHcpSharedVpcPolicyDetails(r, vpcEndpointRoleArn)
if err != nil {
return "", err
}

policyDetails[aws.ControlPlaneCloudCredentialsRoleType] = roles.ManualSharedVpcPolicyDetails{
Command: createPolicyCommand,
Name: policyName,
AlreadyExists: exists,
}
}

var policies []string

// Attach HCP shared VPC policies
switch credrequest {
case aws.IngressOperatorCloudCredentialsRoleType:
policies = append(policies, policyDetails[credrequest].Name)
if !policyDetails[credrequest].AlreadyExists { // Skip creation if already exists
policyCommands = append(policyCommands, policyDetails[credrequest].Command)
// Allow only one creation command for this policy to be printed
if details, ok := policyDetails[credrequest]; ok {
if details, ok := policyDetails[credrequest]; ok {
policies = append(policies, policyDetails[credrequest].Name)
if !policyDetails[credrequest].AlreadyExists { // Skip creation if already exists
policyCommands = append(policyCommands, policyDetails[credrequest].Command)
// Allow only one creation command for this policy to be printed
details.AlreadyExists = true
policyDetails[credrequest] = details
}
}
case aws.ControlPlaneCloudCredentialsRoleType:
for i, details := range policyDetails {
policies = append(policies, details.Name)
if details.AlreadyExists { // Skip creation if already exists
continue
if !details.AlreadyExists {
if details.AlreadyExists { // Skip creation if already exists
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unecessary

Copy link
Contributor

Choose a reason for hiding this comment

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

Being checked in the line above

continue
}
policyCommands = append(policyCommands, details.Command)
// Allow only one creation command for this policy to be printed
details.AlreadyExists = true
policyDetails[i] = details
}
policyCommands = append(policyCommands, details.Command)
// Allow only one creation command for this policy to be printed
details.AlreadyExists = true
policyDetails[i] = details
}
}

Expand Down