Skip to content

Commit

Permalink
OCPBUGS-43453: permissions: add ec2:DescribeInstanceTypeOfferings req
Browse files Browse the repository at this point in the history
The permission is required if the Installer has to derive the available
zones.
  • Loading branch information
r4f4 committed Nov 7, 2024
1 parent c66c051 commit 0d017b1
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 0 deletions.
30 changes: 30 additions & 0 deletions pkg/asset/installconfig/aws/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ const (
// PermissionDeleteIgnitionObjects is a permission set required when `preserveBootstrapIgnition` is not set.
PermissionDeleteIgnitionObjects PermissionGroup = "delete-ignition-objects"

// PermissionDefaultZones is a permission set required when zones are not set in the install-config.
PermissionDefaultZones PermissionGroup = "permission-default-zones"

// PermissionMintCreds is a permission set required when minting credentials.
PermissionMintCreds PermissionGroup = "permission-mint-creds"

Expand Down Expand Up @@ -307,6 +310,12 @@ var permissions = map[PermissionGroup][]string{
// Needed by capa which always deletes the ignition objects once the VMs are up.
"s3:DeleteObject",
},
PermissionDefaultZones: {
// Needed to list the zones available in the region
"ec2:DescribeAvailabilityZones",
// Needed to filter zones by instance type
"ec2:DescribeInstanceTypeOfferings",
},
// From: https://github.com/openshift/cloud-credential-operator/blob/master/pkg/aws/utils.go
// TODO: export these in CCO so we don't have to duplicate them here.
PermissionMintCreds: {
Expand Down Expand Up @@ -486,6 +495,10 @@ func RequiredPermissionGroups(ic *types.InstallConfig) []PermissionGroup {
permissionGroups = append(permissionGroups, PermissionCreateInstanceProfile)
}

if !includesZones(ic) {
permissionGroups = append(permissionGroups, PermissionDefaultZones)
}

return permissionGroups
}

Expand Down Expand Up @@ -624,3 +637,20 @@ func includesCreateInstanceProfile(installConfig *types.InstallConfig) bool {
mpool.Set(installConfig.AWS.DefaultMachinePlatform)
return len(mpool.IAMProfile) == 0
}

// includesZones checks if zones are specified in the install-config. It also returns true if zones will be derived from
// the specified subnets.
func includesZones(installConfig *types.InstallConfig) bool {
mpool := aws.MachinePool{}
mpool.Set(installConfig.AWS.DefaultMachinePlatform)

if mp := installConfig.ControlPlane; mp != nil {
mpool.Set(mp.Platform.AWS)
}

for _, compute := range installConfig.Compute {
mpool.Set(compute.Platform.AWS)
}

return len(mpool.Zones) > 0 || len(installConfig.AWS.Subnets) > 0
}
45 changes: 45 additions & 0 deletions pkg/asset/installconfig/aws/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,3 +732,48 @@ func TestDeleteIgnitionPermissions(t *testing.T) {
assert.NotContains(t, requiredPerms, PermissionDeleteIgnitionObjects)
})
}

func TestIncludesZones(t *testing.T) {
t.Run("Should be true when", func(t *testing.T) {
t.Run("zones specified in defaultMachinePlatform", func(t *testing.T) {
ic := validInstallConfig()
ic.ControlPlane.Platform.AWS.Zones = []string{}
ic.Compute[0].Platform.AWS.Zones = []string{}
ic.AWS.Subnets = []string{}
ic.AWS.DefaultMachinePlatform = &aws.MachinePool{
Zones: []string{"a", "b"},
}
requiredPerms := RequiredPermissionGroups(ic)
assert.NotContains(t, requiredPerms, PermissionDefaultZones)
})
t.Run("zones specified in controlPlane", func(t *testing.T) {
ic := validInstallConfig()
ic.Compute[0].Platform.AWS.Zones = []string{}
ic.AWS.Subnets = []string{}
requiredPerms := RequiredPermissionGroups(ic)
assert.NotContains(t, requiredPerms, PermissionDefaultZones)
})
t.Run("zones specified in compute", func(t *testing.T) {
ic := validInstallConfig()
ic.ControlPlane.Platform.AWS.Zones = []string{}
ic.AWS.Subnets = []string{}
requiredPerms := RequiredPermissionGroups(ic)
assert.NotContains(t, requiredPerms, PermissionDefaultZones)
})
t.Run("subnets specified", func(t *testing.T) {
ic := validInstallConfig()
ic.ControlPlane.Platform.AWS.Zones = []string{}
ic.Compute[0].Platform.AWS.Zones = []string{}
requiredPerms := RequiredPermissionGroups(ic)
assert.NotContains(t, requiredPerms, PermissionDefaultZones)
})
})
t.Run("Should be false when neither zones nor subnets specified", func(t *testing.T) {
ic := validInstallConfig()
ic.AWS.Subnets = []string{}
ic.ControlPlane.Platform.AWS.Zones = []string{}
ic.Compute[0].Platform.AWS.Zones = []string{}
requiredPerms := RequiredPermissionGroups(ic)
assert.Contains(t, requiredPerms, PermissionDefaultZones)
})
}

0 comments on commit 0d017b1

Please sign in to comment.