fix: resolve some of the security group controller errors

.kubernetes/manifests.yaml

@@ -376,13 +376,26 @@ metadata:
   creationTimestamp: null
   name: provider-crossplane-manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - get
+  - list
+  - patch
+  - watch
 - apiGroups:
   - ""
   resources:
   - secrets
   verbs:
+  - create
   - get
   - list
+  - update
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:
@@ -658,7 +671,7 @@ spec:
               name: aws-credentials
         - name: AWS_REGION
           value: us-east-1
-        image: tfgco/provider-crossplane:v0.0.13-alpha
+        image: tfgco/provider-crossplane:v0.1.0-alpha
         livenessProbe:
           httpGet:
             path: /healthz

config/manager/kustomization.yaml

@@ -13,6 +13,6 @@ kind: Kustomization
 images:
 - name: controller
   newName: tfgco/provider-crossplane
-  newTag: v0.0.13-alpha
+  newTag: v0.1.0-alpha
 - name: manager
   newName: tfgco/provider-crossplane

config/rbac/role.yaml

@@ -5,13 +5,26 @@ metadata:
   creationTimestamp: null
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - get
+  - list
+  - patch
+  - watch
 - apiGroups:
   - ""
   resources:
   - secrets
   verbs:
+  - create
   - get
   - list
+  - update
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:

controllers/securitygroup/securitygroup_controller.go

@@ -135,6 +135,8 @@ func (r *SecurityGroupReconciliation) getAWSAccountInfo(ctx context.Context, kcp
 //+kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kopsmachinepools,verbs=get;list;watch
 //+kubebuilder:rbac:groups=controlplane.cluster.x-k8s.io,resources=kopscontrolplanes,verbs=get;list;watch
 //+kubebuilder:rbac:groups=ec2.aws.crossplane.io,resources=securitygroups,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;patch
+//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update
 
 func (c *SecurityGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, rerr error) {
 	r := &SecurityGroupReconciliation{
@@ -313,6 +315,9 @@ func (r *SecurityGroupReconciliation) reconcileKopsMachinePool(ctx context.Conte
 				attachErr = multierror.Append(attachErr, err)
 				continue
 			}
+		} else if kmp.Spec.KopsInstanceGroupSpec.Manager == "Karpenter" {
+			r.Recorder.Eventf(r.sg, corev1.EventTypeWarning, securitygroupv1alpha1.SecurityGroupAttachmentFailedReason, "Karpenter is not supported yet")
+			continue
 		} else {
 			asgName, err := kops.GetCloudResourceNameFromKopsMachinePool(kmp)
 			if err != nil {
@@ -466,7 +471,16 @@ func (r *SecurityGroupReconciliation) reconcileDelete(ctx context.Context, sg *s
 			return resultError, err
 		}
 
-		err := r.detachSGFromKopsMachinePool(ctx, csg, kmp)
+		kcp := &kcontrolplanev1alpha1.KopsControlPlane{}
+		key = client.ObjectKey{
+			Name:      kmp.Spec.ClusterName,
+			Namespace: kmp.ObjectMeta.Namespace,
+		}
+		if err := r.Client.Get(ctx, key, kcp); err != nil {
+			return resultError, err
+		}
+
+		err := r.detachSGFromKopsMachinePool(ctx, csg, kcp, kmp)
 		if err != nil {
 			return resultError, err
 		}
@@ -476,16 +490,16 @@ func (r *SecurityGroupReconciliation) reconcileDelete(ctx context.Context, sg *s
 			Namespace: sg.Spec.InfrastructureRef.Namespace,
 			Name:      sg.Spec.InfrastructureRef.Name,
 		}
+		// TODO: Think how we will handle this when the  kcp is deleted
 		if err := r.Client.Get(ctx, key, kcp); err != nil {
 			return resultError, err
 		}
-
 		kmps, err := kops.GetKopsMachinePoolsWithLabel(ctx, r.Client, "cluster.x-k8s.io/cluster-name", kcp.Name)
 		if err != nil {
 			return resultError, err
 		}
 
-		err = r.detachSGFromKopsMachinePool(ctx, csg, kmps...)
+		err = r.detachSGFromKopsMachinePool(ctx, csg, kcp, kmps...)
 		if err != nil {
 			return resultError, err
 		}
@@ -502,7 +516,14 @@ func (r *SecurityGroupReconciliation) reconcileDelete(ctx context.Context, sg *s
 	return ctrl.Result{}, nil
 }
 
-func (r *SecurityGroupReconciliation) detachSGFromKopsMachinePool(ctx context.Context, csg *crossec2v1beta1.SecurityGroup, kmps ...kinfrastructurev1alpha1.KopsMachinePool) error {
+func (r *SecurityGroupReconciliation) detachSGFromKopsMachinePool(ctx context.Context, csg *crossec2v1beta1.SecurityGroup, kcp *kcontrolplanev1alpha1.KopsControlPlane, kmps ...kinfrastructurev1alpha1.KopsMachinePool) error {
+	_, cfg, err := util.AWSCredentialsFromKCP(ctx, r.Client, kcp)
+	if err != nil {
+		return err
+	}
+
+	r.ec2Client = r.NewEC2ClientFactory(*cfg)
+	r.asgClient = r.NewAutoScalingClientFactory(*cfg)
 
 	var detachErr error
 	for _, kmp := range kmps {
@@ -517,6 +538,9 @@ func (r *SecurityGroupReconciliation) detachSGFromKopsMachinePool(ctx context.Co
 				detachErr = multierror.Append(detachErr, err)
 				continue
 			}
+		} else if kmp.Spec.KopsInstanceGroupSpec.Manager == "Karpenter" {
+			r.Recorder.Eventf(r.sg, corev1.EventTypeWarning, securitygroupv1alpha1.SecurityGroupAttachmentFailedReason, "Karpenter is not supported yet")
+			continue
 		} else {
 			asgName, err := kops.GetCloudResourceNameFromKopsMachinePool(kmp)
 			if err != nil {
@@ -529,7 +553,6 @@ func (r *SecurityGroupReconciliation) detachSGFromKopsMachinePool(ctx context.Co
 				detachErr = multierror.Append(detachErr, err)
 				continue
 			}
-			// TODO: Do we want to detach the SGs from the instances as well?
 		}
 	}
 

controllers/securitygroup/securitygroup_controller_test.go

@@ -726,7 +726,7 @@ func TestReconcileKopsMachinePool(t *testing.T) {
 				return &awsautoscaling.DescribeAutoScalingGroupsOutput{
 					AutoScalingGroups: []autoscalingtypes.AutoScalingGroup{
 						{
-							AutoScalingGroupName: aws.String("testASG"),
+							AutoScalingGroupName: aws.String("test-asg"),
 							LaunchTemplate: &autoscalingtypes.LaunchTemplateSpecification{
 								LaunchTemplateId: aws.String("lt-xxxx"),
 								Version:          aws.String("1"),
@@ -1159,28 +1159,28 @@ func TestReconcileDelete(t *testing.T) {
 		{
 			description: "should remove the crossplane security group referencing kmp",
 			k8sObjects: []client.Object{
-				sg, csg, kcp, kmp, cluster,
+				sg, csg, kcp, kmp, cluster, defaultSecret,
 			},
 			sg: sg,
 		},
 		{
 			description: "should remove the crossplane security group referencing kmp using kops",
 			k8sObjects: []client.Object{
-				sg, csg, kcp, spotKMP, cluster,
+				sg, csg, kcp, spotKMP, cluster, defaultSecret,
 			},
 			sg: sg,
 		},
 		{
 			description: "should remove the crossplane security group referencing kcp",
 			k8sObjects: []client.Object{
-				sgKCP, csg, kcp, kmp, cluster,
+				sgKCP, csg, kcp, kmp, cluster, defaultSecret,
 			},
 			sg: sgKCP,
 		},
 		{
 			description: "should fail with infrastructureRef not supported",
 			k8sObjects: []client.Object{
-				sg, csg, kcp, kmp, cluster,
+				sg, csg, kcp, kmp, cluster, defaultSecret,
 			},
 			sg: &securitygroupv1alpha1.SecurityGroup{
 				ObjectMeta: metav1.ObjectMeta{
@@ -1301,7 +1301,7 @@ func TestReconcileDelete(t *testing.T) {
 				return &awsautoscaling.DescribeAutoScalingGroupsOutput{
 					AutoScalingGroups: []autoscalingtypes.AutoScalingGroup{
 						{
-							AutoScalingGroupName: aws.String("testASG"),
+							AutoScalingGroupName: aws.String("test-asg"),
 							LaunchTemplate: &autoscalingtypes.LaunchTemplateSpecification{
 								LaunchTemplateId: aws.String("lt-xxxx"),
 								Version:          aws.String("1"),
@@ -1484,6 +1484,148 @@ func TestDetachSGFromVNG(t *testing.T) {
 	}
 }
 
+func TestDetachSGFromKopsMachinePool(t *testing.T) {
+	testCases := []struct {
+		description   string
+		k8sObjects    []client.Object
+		csg           *crossec2v1beta1.SecurityGroup
+		kcp           *kcontrolplanev1alpha1.KopsControlPlane
+		kmps          []kinfrastructurev1alpha1.KopsMachinePool
+		errorExpected error
+	}{
+		{
+			description: "should detach sg from kmp",
+			k8sObjects: []client.Object{
+				defaultSecret,
+			},
+			csg: csg,
+			kcp: kcp,
+			kmps: []kinfrastructurev1alpha1.KopsMachinePool{
+				*kmp,
+			},
+		},
+		{
+			description: "should do nothing when kmp manager is karpenter",
+			k8sObjects: []client.Object{
+				defaultSecret,
+			},
+			csg: csg,
+			kcp: kcp,
+			kmps: []kinfrastructurev1alpha1.KopsMachinePool{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: metav1.NamespaceDefault,
+						Name:      "test-kops-machine-pool",
+						Labels: map[string]string{
+							"cluster.x-k8s.io/cluster-name": "test-cluster",
+						},
+					},
+					Spec: kinfrastructurev1alpha1.KopsMachinePoolSpec{
+						ClusterName: "test-cluster",
+						KopsInstanceGroupSpec: kopsapi.InstanceGroupSpec{
+							Manager: "Karpenter",
+							NodeLabels: map[string]string{
+								"kops.k8s.io/instance-group-name": "test-ig",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	RegisterFailHandler(Fail)
+	g := NewWithT(t)
+
+	err := clusterv1beta1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	err = crossec2v1beta1.SchemeBuilder.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	err = securitygroupv1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	err = kinfrastructurev1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	err = kcontrolplanev1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(tc.k8sObjects...).Build()
+
+			fakeEC2Client := &fakeec2.MockEC2Client{}
+			fakeEC2Client.MockDescribeLaunchTemplateVersions = func(ctx context.Context, params *awsec2.DescribeLaunchTemplateVersionsInput, optFns []func(*awsec2.Options)) (*awsec2.DescribeLaunchTemplateVersionsOutput, error) {
+				return &awsec2.DescribeLaunchTemplateVersionsOutput{
+					LaunchTemplateVersions: []ec2types.LaunchTemplateVersion{
+						{
+							LaunchTemplateId: params.LaunchTemplateId,
+							LaunchTemplateData: &ec2types.ResponseLaunchTemplateData{
+								NetworkInterfaces: []ec2types.LaunchTemplateInstanceNetworkInterfaceSpecification{
+									{
+										Groups: []string{
+											"sg-xxxx",
+										},
+									},
+								},
+							},
+						},
+					},
+				}, nil
+			}
+			fakeEC2Client.MockCreateLaunchTemplateVersion = func(ctx context.Context, params *awsec2.CreateLaunchTemplateVersionInput, optFns []func(*awsec2.Options)) (*awsec2.CreateLaunchTemplateVersionOutput, error) {
+				return &awsec2.CreateLaunchTemplateVersionOutput{
+					LaunchTemplateVersion: &ec2types.LaunchTemplateVersion{
+						VersionNumber: aws.Int64(1),
+					},
+				}, nil
+			}
+
+			fakeASGClient := &fakeasg.MockAutoScalingClient{}
+			fakeASGClient.MockDescribeAutoScalingGroups = func(ctx context.Context, params *awsautoscaling.DescribeAutoScalingGroupsInput, optFns []func(*awsautoscaling.Options)) (*awsautoscaling.DescribeAutoScalingGroupsOutput, error) {
+				return &awsautoscaling.DescribeAutoScalingGroupsOutput{
+					AutoScalingGroups: []autoscalingtypes.AutoScalingGroup{
+						{
+							AutoScalingGroupName: aws.String("test-asg"),
+							LaunchTemplate: &autoscalingtypes.LaunchTemplateSpecification{
+								LaunchTemplateId: aws.String("lt-xxxx"),
+								Version:          aws.String("1"),
+							},
+						},
+					},
+				}, nil
+			}
+
+			reconciler := SecurityGroupReconciler{
+				Client:   fakeClient,
+				Recorder: record.NewFakeRecorder(3),
+				NewAutoScalingClientFactory: func(cfg aws.Config) autoscaling.AutoScalingClient {
+					return fakeASGClient
+				},
+				NewEC2ClientFactory: func(cfg aws.Config) ec2.EC2Client {
+					return fakeEC2Client
+				},
+			}
+
+			ctx := context.TODO()
+
+			reconciliation := SecurityGroupReconciliation{
+				SecurityGroupReconciler: reconciler,
+				log:                     ctrl.LoggerFrom(ctx),
+			}
+
+			err = reconciliation.detachSGFromKopsMachinePool(ctx, tc.csg, tc.kcp, tc.kmps...)
+			if tc.errorExpected == nil {
+				g.Expect(err).To(BeNil())
+			} else {
+				g.Expect(err.Error()).To(ContainSubstring(tc.errorExpected.Error()))
+			}
+		})
+	}
+
+}
+
 func TestDetachSGFromASG(t *testing.T) {
 	testCases := []struct {
 		description            string
@@ -1808,7 +1950,7 @@ func TestSecurityGroupStatus(t *testing.T) {
 					return &awsautoscaling.DescribeAutoScalingGroupsOutput{
 						AutoScalingGroups: []autoscalingtypes.AutoScalingGroup{
 							{
-								AutoScalingGroupName: aws.String("testASG"),
+								AutoScalingGroupName: aws.String("test-asg"),
 								LaunchTemplate: &autoscalingtypes.LaunchTemplateSpecification{
 									LaunchTemplateId: aws.String("lt-xxxx"),
 									Version:          aws.String("1"),