Skip to content

Commit 836858d

Browse files
committed
OCPBUGS-36390: aws: do not require create permissions when BYO IAM role
The Installer is unconditionally requiring permissions needed to create IAM roles even when the users use existing roles. They should be included only when needed.
1 parent d0af25c commit 836858d

File tree

2 files changed

+68
-4
lines changed

2 files changed

+68
-4
lines changed

pkg/asset/installconfig/aws/permissions.go

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"github.com/sirupsen/logrus"
1010

1111
ccaws "github.com/openshift/cloud-credential-operator/pkg/aws"
12+
"github.com/openshift/installer/pkg/types"
13+
"github.com/openshift/installer/pkg/types/aws"
1214
)
1315

1416
// PermissionGroup is the group of permissions needed by cluster creation, operation, or teardown.
@@ -30,6 +32,9 @@ const (
3032
// PermissionDeleteSharedNetworking is a set of permissions required when the installer destroys resources from a shared-network cluster.
3133
PermissionDeleteSharedNetworking PermissionGroup = "delete-shared-networking"
3234

35+
// PermissionCreateInstanceRole is a set of permissions required when the installer creates instance roles.
36+
PermissionCreateInstanceRole PermissionGroup = "create-instance-role"
37+
3338
// PermissionDeleteSharedInstanceRole is a set of permissions required when the installer destroys resources from a
3439
// cluster with user-supplied IAM roles for instances.
3540
PermissionDeleteSharedInstanceRole PermissionGroup = "delete-shared-instance-role"
@@ -129,10 +134,7 @@ var permissions = map[PermissionGroup][]string{
129134
// IAM related perms
130135
"iam:AddRoleToInstanceProfile",
131136
"iam:CreateInstanceProfile",
132-
"iam:CreateRole",
133137
"iam:DeleteInstanceProfile",
134-
"iam:DeleteRole",
135-
"iam:DeleteRolePolicy",
136138
"iam:GetInstanceProfile",
137139
"iam:GetRole",
138140
"iam:GetRolePolicy",
@@ -141,7 +143,6 @@ var permissions = map[PermissionGroup][]string{
141143
"iam:ListRoles",
142144
"iam:ListUsers",
143145
"iam:PassRole",
144-
"iam:PutRolePolicy",
145146
"iam:RemoveRoleFromInstanceProfile",
146147
"iam:SimulatePrincipalPolicy",
147148
"iam:TagInstanceProfile",
@@ -246,6 +247,13 @@ var permissions = map[PermissionGroup][]string{
246247
PermissionDeleteSharedNetworking: {
247248
"tag:UnTagResources",
248249
},
250+
// Permissions required for creating an instance role
251+
PermissionCreateInstanceRole: {
252+
"iam:CreateRole",
253+
"iam:DeleteRole",
254+
"iam:DeleteRolePolicy",
255+
"iam:PutRolePolicy",
256+
},
249257
// Permissions required for deleting a cluster with shared instance roles
250258
PermissionDeleteSharedInstanceRole: {
251259
"iam:UntagRole",
@@ -332,3 +340,51 @@ func ValidateCreds(ssn *session.Session, groups []PermissionGroup, region string
332340

333341
return errors.New("AWS credentials cannot be used to either create new creds or use as-is")
334342
}
343+
344+
// IncludesExistingInstanceRole checks if at least one BYO instance role is included in the install-config.
345+
func IncludesExistingInstanceRole(installConfig *types.InstallConfig) bool {
346+
mpool := aws.MachinePool{}
347+
mpool.Set(installConfig.AWS.DefaultMachinePlatform)
348+
349+
if mp := installConfig.ControlPlane; mp != nil {
350+
mpool.Set(mp.Platform.AWS)
351+
}
352+
353+
for _, compute := range installConfig.Compute {
354+
mpool.Set(compute.Platform.AWS)
355+
}
356+
357+
return len(mpool.IAMRole) > 0
358+
}
359+
360+
// IncludesCreateInstanceRole checks if at least one instance role will be created by the installer.
361+
func IncludesCreateInstanceRole(installConfig *types.InstallConfig) bool {
362+
{
363+
mpool := aws.MachinePool{}
364+
mpool.Set(installConfig.AWS.DefaultMachinePlatform)
365+
if mp := installConfig.ControlPlane; mp != nil {
366+
mpool.Set(mp.Platform.AWS)
367+
}
368+
if len(mpool.IAMRole) == 0 {
369+
return true
370+
}
371+
}
372+
373+
for _, compute := range installConfig.Compute {
374+
mpool := aws.MachinePool{}
375+
mpool.Set(installConfig.AWS.DefaultMachinePlatform)
376+
mpool.Set(compute.Platform.AWS)
377+
if len(mpool.IAMRole) == 0 {
378+
return true
379+
}
380+
}
381+
382+
if len(installConfig.Compute) > 0 {
383+
return false
384+
}
385+
386+
// If compute stanza is not defined, we know it'll inherit the value from DefaultMachinePlatform
387+
mpool := aws.MachinePool{}
388+
mpool.Set(installConfig.AWS.DefaultMachinePlatform)
389+
return len(mpool.IAMRole) == 0
390+
}

pkg/asset/installconfig/platformpermscheck.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,14 @@ func (a *PlatformPermsCheck) Generate(ctx context.Context, dependencies asset.Pa
104104
permissionGroups = append(permissionGroups, awsconfig.PermissionDeleteIgnitionObjects)
105105
}
106106

107+
if awsconfig.IncludesCreateInstanceRole(ic.Config) {
108+
permissionGroups = append(permissionGroups, awsconfig.PermissionCreateInstanceRole)
109+
}
110+
111+
if awsconfig.IncludesExistingInstanceRole(ic.Config) {
112+
permissionGroups = append(permissionGroups, awsconfig.PermissionDeleteSharedInstanceRole)
113+
}
114+
107115
ssn, err := ic.AWS.Session(ctx)
108116
if err != nil {
109117
return err

0 commit comments

Comments
 (0)