Repository: keikoproj/instance-manager (go)

Issue:
IGs in special AWS partitions always fails ARN format validation in instance_types.go 
**Is this a BUG REPORT or FEATURE REQUEST?**:
Bug

**What happened**:
Unable to add license specifications and placement configuration to instance_groups for GovCloud resources since ARN prefix validation is hard coded to expect AWS commercial ARN partition.

**What you expected to happen**:
I would expect the instanceGroups be able to successfully process and configure LaunchTemplates regardless of AWS partition.

**How to reproduce it (as minimally and precisely as possible)**:
Configure instanceGroup to have a valid GovCloud ARN in `LicenseSpecifications`, `NotificationArn` and/or `HostResourceGroupArn`. 

**Anything else we need to know?**:

Direct code snippets where the problem is and will occur with the use of `strings.HasPrefix(<arn>, awsprovider.ARNPrefix)` as the method to validate arn formats.

https://github.com/keikoproj/instance-manager/blob/ae893c45a9cdb3ca6bf557b9d73e0abdd782f67a/api/v1alpha1/instancegroup_types.go#L569

https://github.com/keikoproj/instance-manager/blob/ae893c45a9cdb3ca6bf557b9d73e0abdd782f67a/api/v1alpha1/instancegroup_types.go#L572

https://github.com/keikoproj/instance-manager/blob/ae893c45a9cdb3ca6bf557b9d73e0abdd782f67a/api/v1alpha1/instancegroup_types.go#L622

https://github.com/keikoproj/instance-manager/blob/ae893c45a9cdb3ca6bf557b9d73e0abdd782f67a/api/v1alpha1/instancegroup_types.go#L646

https://github.com/keikoproj/instance-manager/blob/ae893c45a9cdb3ca6bf557b9d73e0abdd782f67a/controllers/providers/aws/aws.go#L71

The AWS SDK has the `IsArn` function found in their ARN module (github.com/aws/aws-sdk-go/aws/arn) that should be able to provided the needed facility to validate ARNs.

**Environment**:

- Kubernetes version:
v1.21

**Other debugging information (if applicable)**:

- InstanceGroup status:
```
status:
  configMD5: 23dce88e71ab7fcc8ef222ff636f77d0
  currentState: Error
  lifecycle: normal
  nodesInstanceRoleArn: arn:aws-us-gov:iam::012345678901:role/eks-fake-role-us-gov-west-1
  provisioner: eks
  strategy: crd
```

Relevent manifest configuration details:
```
# ... snip

Spec:
  Eks:
    Configuration:
# ... snip
      licenseSpecifications:
      - arn:aws-us-gov:license-manager:us-gov-west-1:012345678901:license-configuration:lic-0a88baf9a84f39df630da6e67000f88f
      metricsCollection:
      - all
      placement:
        hostResourceGroupArn: arn:aws-us-gov:resource-groups:us-gov-west-1:012345678901:group/ig-1
        tenancy: host
# ... snip
```

- controller logs:

```
controller.instancegroup Reconciler error {reconciler group: instancemgr.keikoproj.io, reconciler kind: InstanceGroup, name: ig-us-gov-west-1a, namespace: instance-manager, error: provisioner eks reconcile failed: validation failed, 'LicenseSpecifications[0]' must be a valid IAM role ARN, errorVerbose: validation failed, 'LicenseSpecifications[0]' must be a valid IAM role ARN
github.com/keikoproj/instance-manager/api/v1alpha1.(*EKSConfiguration).Validate
	/workspace/api/v1alpha1/instancegroup_types.go:623
github.com/keikoproj/instance-manager/api/v1alpha1.(*InstanceGroup).Validate
	/workspace/api/v1alpha1/instancegroup_types.go:741
github.com/keikoproj/instance-manager/controllers.(*InstanceGroupReconciler).Reconcile
	/workspace/controllers/instancegroup_controller.go:197
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:298
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:253
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:214
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1581
provisioner eks reconcile failed
github.com/keikoproj/instance-manager/controllers.(*InstanceGroupReconciler).Reconcile
	/workspace/controllers/instancegroup_controller.go:201
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:298
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:253
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.9.7/pkg/internal/controller/controller.go:214
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1581}
```


Pull request:
fix/373 Replace ARN format validation procedure
Fixes #373 

Feedback is welcome.

```
coverage: 83.8% of statements
ok      github.com/keikoproj/instance-manager/controllers/provisioners/eksmanaged       0.057s  coverage: 83.8% of statements
=== RUN   TestInstanceGroupSpecValidate
=== RUN   TestInstanceGroupSpecValidate/eks-fargate_with_managed_strategy
=== RUN   TestInstanceGroupSpecValidate/eks-bogus_provisioner
=== RUN   TestInstanceGroupSpecValidate/eks-fargate_with_bad_strategy
=== RUN   TestInstanceGroupSpecValidate/eks_with_empty_strings_in_licenseSpecifications
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_licenseSpecification
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_container_runtime
=== RUN   TestInstanceGroupSpecValidate/eks_with_valid_Placement
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_combination_of_HostResourceGroupArn_and_Tenancy_in_Placement
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_HostResourceGroupArn_in_Placement
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_Tenancy_in_Placement
=== RUN   TestInstanceGroupSpecValidate/eks_with_gp3_volume_validates
=== RUN   TestInstanceGroupSpecValidate/eks_with_gp2_volume_with_provisioned_throughput_fails
=== RUN   TestInstanceGroupSpecValidate/eks_with_gp2_volume_with_provisioned_iops_fails
=== RUN   TestInstanceGroupSpecValidate/eks_with_metadataoptions_validates
=== RUN   TestInstanceGroupSpecValidate/eks_with_valid_configuration_for_any_random_partition
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_LicenseSpecifications_for_any_random_partition
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_combination_of_HostResourceGroupArn_and_Tenancy_in_Placement_for_any_random_partition
=== RUN   TestInstanceGroupSpecValidate/eks_with_invalid_HostResourceGroupArn_in_Placement_for_any_random_partition
--- PASS: TestInstanceGroupSpecValidate (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks-fargate_with_managed_strategy (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks-bogus_provisioner (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks-fargate_with_bad_strategy (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_empty_strings_in_licenseSpecifications (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_licenseSpecification (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_container_runtime (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_valid_Placement (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_combination_of_HostResourceGroupArn_and_Tenancy_in_Placement (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_HostResourceGroupArn_in_Placement (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_Tenancy_in_Placement (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_gp3_volume_validates (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_gp2_volume_with_provisioned_throughput_fails (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_gp2_volume_with_provisioned_iops_fails (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_metadataoptions_validates (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_valid_configuration_for_any_random_partition (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_LicenseSpecifications_for_any_random_partition (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_combination_of_HostResourceGroupArn_and_Tenancy_in_Placement_for_any_random_partition (0.00s)
    --- PASS: TestInstanceGroupSpecValidate/eks_with_invalid_HostResourceGroupArn_in_Placement_for_any_random_partition (0.00s)
=== RUN   TestLockedAnnotation
=== RUN   TestLockedAnnotation/Locked
=== RUN   TestLockedAnnotation/Unlocked
--- PASS: TestLockedAnnotation (0.00s)
    --- PASS: TestLockedAnnotation/Locked (0.00s)
    --- PASS: TestLockedAnnotation/Unlocked (0.00s)
PASS
coverage: 13.0% of statements
ok      github.com/keikoproj/instance-manager/api/v1alpha1      0.074s  coverage: 13.0% of statements
go build -o bin/manager main.go

```

Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index 70a8af3..9eaee38 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -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"
 
@@ -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)
@@ -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)
 		}
 	}
@@ -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")
 	}
 
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index 8896885..bd8d4f3 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -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"
 )
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index da3076f..7d9a2c5 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -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"
@@ -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))


Test patch:
diff --git a/api/v1alpha1/instancegroup_types_test.go b/api/v1alpha1/instancegroup_types_test.go
index 4d94d99..7581aec 100644
--- a/api/v1alpha1/instancegroup_types_test.go
+++ b/api/v1alpha1/instancegroup_types_test.go
@@ -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) {