Authorization Controller for NodeClass

pkg/apis/v1/ec2nodeclass_status.go

@@ -24,6 +24,7 @@ const (
 	ConditionTypeSecurityGroupsReady  = "SecurityGroupsReady"
 	ConditionTypeAMIsReady            = "AMIsReady"
 	ConditionTypeInstanceProfileReady = "InstanceProfileReady"
+	ConditionTypeAuthorization        = "AuthorizationReady"
 	ConditionTypeValidationSucceeded  = "ValidationSucceeded"
 )
 
@@ -94,6 +95,7 @@ func (in *EC2NodeClass) StatusConditions() status.ConditionSet {
 		ConditionTypeSubnetsReady,
 		ConditionTypeSecurityGroupsReady,
 		ConditionTypeInstanceProfileReady,
+		ConditionTypeAuthorization,
 		ConditionTypeValidationSucceeded,
 	).For(in)
 }

pkg/cloudprovider/cloudprovider.go

@@ -35,6 +35,7 @@ import (
 
 	"github.com/aws/karpenter-provider-aws/pkg/apis"
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
 	"github.com/aws/karpenter-provider-aws/pkg/operator/options"
 	"github.com/aws/karpenter-provider-aws/pkg/utils"
 
@@ -93,7 +94,7 @@ func (c *CloudProvider) Create(ctx context.Context, nodeClaim *karpv1.NodeClaim)
 	if nodeClassReady.IsFalse() {
 		return nil, cloudprovider.NewNodeClassNotReadyError(stderrors.New(nodeClassReady.Message))
 	}
-	if nodeClassReady.IsUnknown() {
+	if nodeClassReady.IsUnknown() && ctx.Value("DryRun") == nil {
 		return nil, cloudprovider.NewCreateError(fmt.Errorf("resolving NodeClass readiness, NodeClass is in Ready=Unknown, %s", nodeClassReady.Message), "NodeClass is in Ready=Unknown")
 	}
 	instanceTypes, err := c.resolveInstanceTypes(ctx, nodeClaim, nodeClass)
@@ -109,6 +110,9 @@ func (c *CloudProvider) Create(ctx context.Context, nodeClaim *karpv1.NodeClaim)
 	}
 	instance, err := c.instanceProvider.Create(ctx, nodeClass, nodeClaim, tags, instanceTypes)
 	if err != nil {
+		if awserrors.IsUnauthorizedError(err) {
+			return nil, cloudprovider.NewNodeClassNotReadyError(err)
+		}
 		conditionMessage := "Error creating instance"
 		var createError *cloudprovider.CreateError
 		if stderrors.As(err, &createError) {

pkg/cloudprovider/suite_test.go

@@ -1156,7 +1156,7 @@ var _ = Describe("CloudProvider", func() {
 				{SubnetId: aws.String("test-subnet-2"), AvailabilityZone: aws.String("test-zone-1a"), AvailabilityZoneId: aws.String("tstz1-1a"), AvailableIpAddressCount: aws.Int32(100),
 					Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}},
 			}})
-			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
+			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider, *cloudProvider, awsEnv.InstanceProvider)
 			ExpectApplied(ctx, env.Client, nodePool, nodeClass)
 			ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
 			pod := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{corev1.LabelTopologyZone: "test-zone-1a"}})
@@ -1173,7 +1173,7 @@ var _ = Describe("CloudProvider", func() {
 				{SubnetId: aws.String("test-subnet-2"), AvailabilityZone: aws.String("test-zone-1a"), AvailabilityZoneId: aws.String("tstz1-1a"), AvailableIpAddressCount: aws.Int32(11),
 					Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}},
 			}})
-			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
+			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider, *cloudProvider, awsEnv.InstanceProvider)
 			nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{
 				MaxPods: aws.Int32(1),
 			}
@@ -1214,7 +1214,7 @@ var _ = Describe("CloudProvider", func() {
 			}})
 			nodeClass.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{{Tags: map[string]string{"Name": "test-subnet-1"}}}
 			ExpectApplied(ctx, env.Client, nodePool, nodeClass)
-			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
+			controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider, *cloudProvider, awsEnv.InstanceProvider)
 			ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
 			podSubnet1 := coretest.UnschedulablePod()
 			ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, podSubnet1)

pkg/controllers/controllers.go

@@ -41,12 +41,14 @@ import (
 	servicesqs "github.com/aws/aws-sdk-go-v2/service/sqs"
 	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/clock"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/karpenter/pkg/events"
 
 	awscache "github.com/aws/karpenter-provider-aws/pkg/cache"
+	provcloudprovider "github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
 	"github.com/aws/karpenter-provider-aws/pkg/controllers/interruption"
 	nodeclaimgarbagecollection "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclaim/garbagecollection"
 	nodeclaimtagging "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclaim/tagging"
@@ -82,7 +84,8 @@ func NewControllers(
 	instanceTypeProvider *instancetype.DefaultProvider) []controller.Controller {
 	controllers := []controller.Controller{
 		nodeclasshash.NewController(kubeClient),
-		nodeclassstatus.NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, instanceProfileProvider, launchTemplateProvider),
+		nodeclassstatus.NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, instanceProfileProvider, launchTemplateProvider, *provcloudprovider.New(instanceTypeProvider, instanceProvider, events.NewRecorder(&record.FakeRecorder{}),
+			kubeClient, amiProvider, securityGroupProvider), instanceProvider),
 		nodeclasstermination.NewController(kubeClient, recorder, instanceProfileProvider, launchTemplateProvider),
 		nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider),
 		nodeclaimtagging.NewController(kubeClient, cloudProvider, instanceProvider),

pkg/controllers/nodeclass/status/authorization.go

@@ -0,0 +1,61 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package status
+
+import (
+	"context"
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
+	coretest "sigs.k8s.io/karpenter/pkg/test"
+
+	"github.com/samber/lo"
+
+	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
+	"github.com/aws/karpenter-provider-aws/pkg/providers/instance"
+)
+
+type Authorization struct {
+	cloudProvider    cloudprovider.CloudProvider
+	instanceProvider instance.Provider
+}
+
+func (a Authorization) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconcile.Result, error) {
+	//nolint:ineffassign, staticcheck
+	ctx = context.WithValue(ctx, "DryRun", lo.ToPtr(true))
+	if nodeClass.StatusConditions().Get(v1.ConditionTypeSubnetsReady).IsFalse() || nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsReady).IsFalse() {
+		return reconcile.Result{}, nil
+	}
+	nodeClaim := coretest.NodeClaim()
+	nodeClaim.Spec.NodeClassRef.Name = nodeClass.Name
+	_, err := a.cloudProvider.Create(ctx, nodeClaim)
+	if err == nil {
+		err = a.instanceProvider.Delete(ctx, "mock-id")
+		if err == nil {
+			err = a.instanceProvider.CreateTags(ctx, "mock-id", map[string]string{"mock-tag": "mock-tag-value"})
+		}
+	}
+	//nolint:ineffassign, staticcheck
+	ctx = context.WithValue(ctx, "DryRun", lo.ToPtr(false))
+	if corecloudprovider.IsNodeClassNotReadyError(err) {
+		nodeClass.StatusConditions().SetFalse(v1.ConditionTypeAuthorization, "NodeClassNotReady", "Unauthorized Operation")
+		return reconcile.Result{}, fmt.Errorf("unauthorized operation %w", err)
+	}
+	nodeClass.StatusConditions().SetTrue(v1.ConditionTypeAuthorization)
+	return reconcile.Result{}, nil
+}

pkg/controllers/nodeclass/status/authorization_test.go

@@ -0,0 +1,70 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package status_test
+
+import (
+	"github.com/aws/smithy-go"
+
+	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	"github.com/aws/karpenter-provider-aws/pkg/fake"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	. "sigs.k8s.io/karpenter/pkg/test/expectations"
+)
+
+var _ = Describe("NodeClass Authorized Status Controller", func() {
+	It("should update status condition on nodeClass as NotReady when nodeclass has authorization failure due to createfleet", func() {
+		ExpectApplied(ctx, env.Client, nodeClass)
+		awsEnv.EC2API.CreateFleetBehavior.Error.Set(&smithy.GenericAPIError{
+			Code: "UnauthorizedOperation",
+		}, fake.MaxCalls(1))
+		err := ExpectObjectReconcileFailed(ctx, env.Client, statusController, nodeClass)
+		Expect(err).To(HaveOccurred())
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.Status.Conditions).To(HaveLen(7))
+		Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAuthorization).IsFalse()).To(BeTrue())
+	})
+	It("should update status condition on nodeClass as NotReady when nodeclass has authorization failure due to terminateinstances", func() {
+		ExpectApplied(ctx, env.Client, nodeClass)
+		awsEnv.EC2API.TerminateInstancesBehavior.Error.Set(&smithy.GenericAPIError{
+			Code: "UnauthorizedOperation",
+		}, fake.MaxCalls(2))
+		err := ExpectObjectReconcileFailed(ctx, env.Client, statusController, nodeClass)
+		Expect(err).To(HaveOccurred())
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.Status.Conditions).To(HaveLen(7))
+		Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAuthorization).IsFalse()).To(BeTrue())
+	})
+	It("should update status condition on nodeClass as NotReady when nodeclass has authorization failure due to CreateTags", func() {
+		ExpectApplied(ctx, env.Client, nodeClass)
+		awsEnv.EC2API.CreateTagsBehavior.Error.Set(&smithy.GenericAPIError{
+			Code: "UnauthorizedOperation",
+		}, fake.MaxCalls(1))
+		err := ExpectObjectReconcileFailed(ctx, env.Client, statusController, nodeClass)
+		Expect(err).To(HaveOccurred())
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.Status.Conditions).To(HaveLen(7))
+		Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAuthorization).IsFalse()).To(BeTrue())
+	})
+	It("should update status condition as Ready", func() {
+		nodeClass.Spec.Tags = map[string]string{}
+		ExpectApplied(ctx, env.Client, nodeClass)
+		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+
+		Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAuthorization).IsTrue()).To(BeTrue())
+	})
+})

pkg/controllers/nodeclass/status/controller.go

@@ -33,7 +33,9 @@ import (
 	"github.com/awslabs/operatorpkg/reasonable"
 
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/amifamily"
+	"github.com/aws/karpenter-provider-aws/pkg/providers/instance"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/instanceprofile"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup"
@@ -52,11 +54,12 @@ type Controller struct {
 	subnet          *Subnet
 	securitygroup   *SecurityGroup
 	validation      *Validation
+	authorization   *Authorization
 	readiness       *Readiness //TODO : Remove this when we have sub status conditions
 }
 
 func NewController(kubeClient client.Client, subnetProvider subnet.Provider, securityGroupProvider securitygroup.Provider,
-	amiProvider amifamily.Provider, instanceProfileProvider instanceprofile.Provider, launchTemplateProvider launchtemplate.Provider) *Controller {
+	amiProvider amifamily.Provider, instanceProfileProvider instanceprofile.Provider, launchTemplateProvider launchtemplate.Provider, cloudProvider cloudprovider.CloudProvider, instanceProvider instance.Provider) *Controller {
 	return &Controller{
 		kubeClient: kubeClient,
 
@@ -65,6 +68,7 @@ func NewController(kubeClient client.Client, subnetProvider subnet.Provider, sec
 		securitygroup:   &SecurityGroup{securityGroupProvider: securityGroupProvider},
 		instanceprofile: &InstanceProfile{instanceProfileProvider: instanceProfileProvider},
 		validation:      &Validation{},
+		authorization:   &Authorization{cloudProvider: cloudProvider, instanceProvider: instanceProvider},
 		readiness:       &Readiness{launchTemplateProvider: launchTemplateProvider},
 	}
 }
@@ -96,6 +100,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass)
 		c.securitygroup,
 		c.instanceprofile,
 		c.validation,
+		c.authorization,
 		c.readiness,
 	} {
 		res, err := reconciler.Reconcile(ctx, nodeClass)

pkg/controllers/nodeclass/status/readiness_test.go

@@ -53,7 +53,7 @@ var _ = Describe("NodeClass Status Condition Controller", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
-		Expect(nodeClass.Status.Conditions).To(HaveLen(6))
+		Expect(nodeClass.Status.Conditions).To(HaveLen(7))
 		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsTrue()).To(BeTrue())
 	})
 	It("should update status condition as Not Ready", func() {

pkg/controllers/nodeclass/status/suite_test.go

@@ -23,8 +23,11 @@ import (
 	coreoptions "sigs.k8s.io/karpenter/pkg/operator/options"
 	coretest "sigs.k8s.io/karpenter/pkg/test"
 
+	"github.com/samber/lo"
+
 	"github.com/aws/karpenter-provider-aws/pkg/apis"
 	v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
+	"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
 	"github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status"
 	"github.com/aws/karpenter-provider-aws/pkg/operator/options"
 	"github.com/aws/karpenter-provider-aws/pkg/test"
@@ -40,6 +43,7 @@ var env *coretest.Environment
 var awsEnv *test.Environment
 var nodeClass *v1.EC2NodeClass
 var statusController *status.Controller
+var cloudProvider cloudprovider.CloudProvider
 
 func TestAPIs(t *testing.T) {
 	ctx = TestContextWithLogger(t)
@@ -53,13 +57,23 @@ var _ = BeforeSuite(func() {
 	ctx = options.ToContext(ctx, test.Options())
 	awsEnv = test.NewEnvironment(ctx, env)
 
+	instanceTypesProvider := awsEnv.InstanceTypesProvider
+
+	Expect(instanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed())
+	Expect(instanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed())
+
+	cloudProvider = lo.FromPtr(cloudprovider.New(instanceTypesProvider, awsEnv.InstanceProvider, coretest.NewEventRecorder(),
+		env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider))
+
 	statusController = status.NewController(
 		env.Client,
 		awsEnv.SubnetProvider,
 		awsEnv.SecurityGroupProvider,
 		awsEnv.AMIProvider,
 		awsEnv.InstanceProfileProvider,
 		awsEnv.LaunchTemplateProvider,
+		cloudProvider,
+		awsEnv.InstanceProvider,
 	)
 })
 
@@ -71,6 +85,25 @@ var _ = BeforeEach(func() {
 	ctx = coreoptions.ToContext(ctx, coretest.Options())
 	nodeClass = test.EC2NodeClass()
 	awsEnv.Reset()
+
+	instanceTypesProvider := awsEnv.InstanceTypesProvider
+
+	Expect(instanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed())
+	Expect(instanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed())
+
+	cloudProvider = lo.FromPtr(cloudprovider.New(instanceTypesProvider, awsEnv.InstanceProvider, coretest.NewEventRecorder(),
+		env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider))
+
+	statusController = status.NewController(
+		env.Client,
+		awsEnv.SubnetProvider,
+		awsEnv.SecurityGroupProvider,
+		awsEnv.AMIProvider,
+		awsEnv.InstanceProfileProvider,
+		awsEnv.LaunchTemplateProvider,
+		cloudProvider,
+		awsEnv.InstanceProvider,
+	)
 })
 
 var _ = AfterEach(func() {

pkg/controllers/nodeclass/status/validation_test.go

@@ -58,7 +58,7 @@ var _ = Describe("NodeClass Validation Status Controller", func() {
 		err := ExpectObjectReconcileFailed(ctx, env.Client, statusController, nodeClass)
 		Expect(err).To(HaveOccurred())
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
-		Expect(nodeClass.Status.Conditions).To(HaveLen(6))
+		Expect(nodeClass.Status.Conditions).To(HaveLen(7))
 		Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeValidationSucceeded).IsFalse()).To(BeTrue())
 		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue())
 		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("ValidationSucceeded=False"))

pkg/errors/errors.go

@@ -16,6 +16,7 @@ package errors
 
 import (
 	"errors"
+	"strings"
 
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/aws/smithy-go"
@@ -106,3 +107,16 @@ func IsLaunchTemplateNotFound(err error) bool {
 	}
 	return false
 }
+
+func IsUnauthorizedError(err error) bool {
+	if err == nil {
+		return false
+	}
+	var apiErr smithy.APIError
+	if errors.As(err, &apiErr) {
+		return apiErr.ErrorCode() == "UnauthorizedOperation"
+	}
+
+	return strings.Contains(err.Error(), "UnauthorizedOperation")
+
+}

pkg/fake/ec2api.go

@@ -217,7 +217,9 @@ func (e *EC2API) CreateLaunchTemplate(_ context.Context, input *ec2.CreateLaunch
 		defer e.NextError.Reset()
 		return nil, e.NextError.Get()
 	}
-	e.CalledWithCreateLaunchTemplateInput.Add(input)
+	if !*input.DryRun {
+		e.CalledWithCreateLaunchTemplateInput.Add(input)
+	}
 	launchTemplate := ec2types.LaunchTemplate{LaunchTemplateName: input.LaunchTemplateName}
 	e.LaunchTemplates.Store(input.LaunchTemplateName, launchTemplate)
 	return &ec2.CreateLaunchTemplateOutput{LaunchTemplate: lo.ToPtr(launchTemplate)}, nil