Skip to content

Commit

Permalink
fix/373 Replace ARN format validation procedure
Browse files Browse the repository at this point in the history 
Signed-off-by: Shaunn Rosenberg <shaunnrosenberg@gmail.com>
Signed-off-by: Shaunn Rosenberg <srosenberg@apptio.com>
  • Loading branch information
@srosenberg-apptio
srosenberg-apptio committed Nov 4, 2022
1 parent ae893c4 commit 4ab98e3
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 9 deletions.
9 changes: 5 additions & 4 deletions api/v1alpha1/instancegroup_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"reflect"
"strings"

"github.com/aws/aws-sdk-go/aws/arn"
"github.com/keikoproj/instance-manager/controllers/common"
awsprovider "github.com/keikoproj/instance-manager/controllers/providers/aws"

Expand Down Expand Up @@ -566,10 +567,10 @@ func (c *EKSConfiguration) Validate() error {
if common.StringEmpty(h.Name) {
return errors.Errorf("validation failed, 'name' is a required parameter")
}
if !common.StringEmpty(h.NotificationArn) && !strings.HasPrefix(h.NotificationArn, awsprovider.ARNPrefix) {
if !common.StringEmpty(h.NotificationArn) && !arn.IsARN(h.NotificationArn) {
return errors.Errorf("validation failed, 'notificationArn' must be a valid IAM role ARN")
}
if !common.StringEmpty(h.RoleArn) && !strings.HasPrefix(h.NotificationArn, awsprovider.ARNPrefix) {
if !common.StringEmpty(h.RoleArn) && !arn.IsARN(h.NotificationArn) {
return errors.Errorf("validation failed, 'roleArn' must be a valid IAM role ARN")
}
hooks = append(hooks, h)
Expand Down Expand Up @@ -619,7 +620,7 @@ func (c *EKSConfiguration) Validate() error {
}

for i, v := range c.LicenseSpecifications {
if !strings.HasPrefix(v, awsprovider.ARNPrefix) {
if !arn.IsARN(v) {
return errors.Errorf("validation failed, 'LicenseSpecifications[%d]' must be a valid IAM role ARN", i)
}
}
Expand All @@ -643,7 +644,7 @@ func (p *PlacementSpec) Validate() error {
return errors.Errorf("validation failed, Tenancy must be one of default, dedicated, host")
}

if !common.StringEmpty(p.HostResourceGroupArn) && !strings.HasPrefix(p.HostResourceGroupArn, awsprovider.ARNPrefix) {
if !common.StringEmpty(p.HostResourceGroupArn) && !arn.IsARN(p.HostResourceGroupArn) {
return errors.Errorf("validation failed, HostResourceGroupArn must be a valid dedicated HostResourceGroup ARN")
}

Expand Down
91 changes: 91 additions & 0 deletions api/v1alpha1/instancegroup_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,97 @@ func TestInstanceGroupSpecValidate(t *testing.T) {
},
want: "",
},
{
name: "eks with valid configuration for any random partition",
args: args{
instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
MaxSize: 1,
MinSize: 1,
Type: "LaunchTemplate",
EKSConfiguration: &EKSConfiguration{
EksClusterName: "my-eks-cluster",
NodeSecurityGroups: []string{"sg-123456789"},
Image: "ami-12345",
InstanceType: "m5.large",
KeyPairName: "thisShouldBeOptional",
Subnets: []string{"subnet-1111111", "subnet-222222"},
LicenseSpecifications: []string{"arn:some-partition:some-service:somewhere-west-1:some-account-number:license-configuration:lic-0a88baf9a84f39df630da6e67000f88f"},
Placement: &PlacementSpec{
AvailabilityZone: "us-gov-west-1a",
HostResourceGroupArn: "arn:some-partition:some-group:somewhere-west-1:some-account-number:group/resourceName",
Tenancy: "host",
},
},
}, nil, nil),
},
want: "",
},
{
name: "eks with invalid LicenseSpecifications for any random partition",
args: args{
instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
MaxSize: 1,
MinSize: 1,
Type: "LaunchTemplate",
EKSConfiguration: &EKSConfiguration{
EksClusterName: "my-eks-cluster",
NodeSecurityGroups: []string{"sg-123456789"},
Image: "ami-12345",
InstanceType: "m5.large",
KeyPairName: "thisShouldBeOptional",
Subnets: []string{"subnet-1111111", "subnet-222222"},
LicenseSpecifications: []string{"arn:missingfield:license-manager012345678901:license-configuration:lic-0a88baf9a84f39df630da6e67000f88f"},
},
}, nil, nil),
},
want: "validation failed, 'LicenseSpecifications[0]' must be a valid IAM role ARN",
},
{
name: "eks with invalid combination of HostResourceGroupArn and Tenancy in Placement for any random partition",
args: args{
instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
MaxSize: 1,
MinSize: 1,
Type: "LaunchTemplate",
EKSConfiguration: &EKSConfiguration{
EksClusterName: "my-eks-cluster",
NodeSecurityGroups: []string{"sg-123456789"},
Image: "ami-12345",
InstanceType: "m5.large",
KeyPairName: "thisShouldBeOptional",
Subnets: []string{"subnet-1111111", "subnet-222222"},
Placement: &PlacementSpec{
HostResourceGroupArn: "arn:some-partition:resource-groups:us-west-2:1122334455:group/resourceName",
Tenancy: "default",
},
},
}, nil, nil),
},
want: "validation failed, Tenancy must be \"host\" when HostResourceGroupArn is set",
},
{
name: "eks with invalid HostResourceGroupArn in Placement for any random partition",
args: args{
instancegroup: MockInstanceGroup("eks", "rollingUpdate", &EKSSpec{
MaxSize: 1,
MinSize: 1,
Type: "LaunchTemplate",
EKSConfiguration: &EKSConfiguration{
EksClusterName: "my-eks-cluster",
NodeSecurityGroups: []string{"sg-123456789"},
Image: "ami-12345",
InstanceType: "m5.large",
KeyPairName: "thisShouldBeOptional",
Subnets: []string{"subnet-1111111", "subnet-222222"},
Placement: &PlacementSpec{
HostResourceGroupArn: "arn:missing-field:resource-groups1122334455:group/resourceName",
Tenancy: "host",
},
},
}, nil, nil),
},
want: "validation failed, HostResourceGroupArn must be a valid dedicated HostResourceGroup ARN",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
2 changes: 0 additions & 2 deletions controllers/providers/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ const (
LaunchTemplateAllocationStrategy = "prioritized"
LaunchTemplateLatestVersionKey = "$Latest"
IAMPolicyPrefix = "arn:aws:iam::aws:policy"
IAMARNPrefix = "arn:aws:iam::"
ARNPrefix = "arn:aws:"
LaunchConfigurationNotFoundErrorMessage = "Launch configuration name not found"
defaultPolicyArn = "arn:aws:iam::aws:policy/AmazonEKSFargatePodExecutionRolePolicy"
)
Expand Down
5 changes: 2 additions & 3 deletions controllers/provisioners/eks/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/Masterminds/semver"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/autoscaling"
"github.com/keikoproj/instance-manager/api/v1alpha1"
"github.com/keikoproj/instance-manager/controllers/common"
Expand Down Expand Up @@ -1027,9 +1028,7 @@ func (ctx *EksInstanceGroupContext) GetManagedPoliciesList(additionalPolicies []
managedPolicies := make([]string, 0)
for _, name := range additionalPolicies {
switch {
case strings.HasPrefix(name, awsprovider.IAMPolicyPrefix):
managedPolicies = append(managedPolicies, name)
case strings.HasPrefix(name, awsprovider.IAMARNPrefix):
case arn.IsARN(name):
managedPolicies = append(managedPolicies, name)
default:
managedPolicies = append(managedPolicies, fmt.Sprintf("%s/%s", awsprovider.IAMPolicyPrefix, name))
Expand Down

0 comments on commit 4ab98e3

Please sign in to comment.