fix(ec2): Possible nil derefs in ec2 controllers

* vpcendpoint * vpcendpointconfiguration * vpcpeeringconnection Signed-off-by: Maximilian Blatt (external expert on behalf of DB Netz) <[email protected]> (cherry picked from commit 911508b)
crossplane-contrib · Sep 6, 2023 · 05aaba3 · 05aaba3
1 parent d5f6675
commit 05aaba3
Show file tree

Hide file tree

Showing 6 changed files with 552 additions and 1 deletion.
diff --git a/pkg/controller/ec2/vpcendpoint/setup.go b/pkg/controller/ec2/vpcendpoint/setup.go
@@ -372,12 +372,21 @@ func (t *tagger) Initialize(ctx context.Context, mgd cpresource.Managed) error {
 	}
 	var vpcEndpointTags svcapitypes.TagSpecification
 	for _, tagSpecification := range cr.Spec.ForProvider.TagSpecifications {
+		if tagSpecification == nil {
+			continue
+		}
 		if aws.StringValue(tagSpecification.ResourceType) == "vpc-endpoint" {
 			vpcEndpointTags = *tagSpecification
 		}
 	}
 
-	tagMap := cr.Spec.ForProvider.Tags
+	var tagMap map[string]string
+	if cr.Spec.ForProvider.Tags != nil {
+		tagMap = cr.Spec.ForProvider.Tags
+	} else {
+		tagMap = map[string]string{}
+	}
+
 	tagMap["Name"] = cr.Name
 	for k, v := range cpresource.GetExternalTags(mgd) {
 		tagMap[k] = v

diff --git a/pkg/controller/ec2/vpcendpoint/setup_test.go b/pkg/controller/ec2/vpcendpoint/setup_test.go
@@ -1,3 +1,19 @@
+/*
+Copyright 2023 The Crossplane Authors.
+
+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 vpcendpoint
 
 import (
@@ -9,10 +25,12 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/crossplane/crossplane-runtime/pkg/meta"
 	"github.com/crossplane/crossplane-runtime/pkg/reconciler/managed"
+	cpresource "github.com/crossplane/crossplane-runtime/pkg/resource"
 	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/crossplane-contrib/provider-aws/apis/ec2/v1alpha1"
@@ -44,6 +62,10 @@ type args struct {
 
 type vpcEndpointModifier func(*v1alpha1.VPCEndpoint)
 
+func withName(name string) vpcEndpointModifier {
+	return func(r *v1alpha1.VPCEndpoint) { r.SetName(name) }
+}
+
 func withExternalName(name string) vpcEndpointModifier {
 	return func(r *v1alpha1.VPCEndpoint) { meta.SetExternalName(r, name) }
 }
@@ -555,3 +577,138 @@ func TestObserve(t *testing.T) {
 		})
 	}
 }
+
+func TestTagger(t *testing.T) {
+	type want struct {
+		cr  *v1alpha1.VPCEndpoint
+		err error
+	}
+
+	tag := func(k, v string) *v1alpha1.Tag {
+		return &v1alpha1.Tag{Key: pointer.String(k), Value: pointer.String(v)}
+	}
+
+	cases := map[string]struct {
+		args
+		want
+	}{
+		"ShouldAddTagsIfSpecIsNil": {
+			args: args{
+				kube: &test.MockClient{
+					MockUpdate: test.NewMockUpdateFn(nil),
+				},
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{}),
+				),
+			},
+			want: want{
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{
+						TagSpecifications: []*v1alpha1.TagSpecification{
+							{
+								ResourceType: aws.String("vpc-endpoint"),
+								Tags: []*v1alpha1.Tag{
+									tag("Name", "test"),
+									tag(cpresource.ExternalResourceTagKeyKind, ""),
+									tag(cpresource.ExternalResourceTagKeyName, "test"),
+								},
+							},
+						},
+					}),
+				),
+			},
+		},
+		"ShouldOverwriteTags": {
+			args: args{
+				kube: &test.MockClient{
+					MockUpdate: test.NewMockUpdateFn(nil),
+				},
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{
+						TagSpecifications: []*v1alpha1.TagSpecification{
+							{
+								ResourceType: aws.String("vpc-endpoint"),
+								Tags: []*v1alpha1.Tag{
+									tag(cpresource.ExternalResourceTagKeyName, "preset"),
+								},
+							},
+						},
+					}),
+				),
+			},
+			want: want{
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{
+						TagSpecifications: []*v1alpha1.TagSpecification{
+							{
+								ResourceType: aws.String("vpc-endpoint"),
+								Tags: []*v1alpha1.Tag{
+									tag("Name", "test"),
+									tag(cpresource.ExternalResourceTagKeyKind, ""),
+									tag(cpresource.ExternalResourceTagKeyName, "test"),
+								},
+							},
+						},
+					}),
+				),
+			},
+		},
+		"ShouldMergeTags": {
+			args: args{
+				kube: &test.MockClient{
+					MockUpdate: test.NewMockUpdateFn(nil),
+				},
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{
+						TagSpecifications: []*v1alpha1.TagSpecification{
+							{
+								ResourceType: aws.String("vpc-endpoint"),
+								Tags: []*v1alpha1.Tag{
+									tag("Name", "test"),
+									tag(cpresource.ExternalResourceTagKeyKind, ""),
+									tag(cpresource.ExternalResourceTagKeyName, "test"),
+								},
+							},
+						},
+					}),
+				),
+			},
+			want: want{
+				cr: vpcEndpoint(
+					withName("test"),
+					withSpec(v1alpha1.VPCEndpointParameters{
+						TagSpecifications: []*v1alpha1.TagSpecification{
+							{
+								ResourceType: aws.String("vpc-endpoint"),
+								Tags: []*v1alpha1.Tag{
+									tag("Name", "test"),
+									tag(cpresource.ExternalResourceTagKeyKind, ""),
+									tag(cpresource.ExternalResourceTagKeyName, "test"),
+								},
+							},
+						},
+					}),
+				),
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			ta := tagger{kube: tc.args.kube}
+			err := ta.Initialize(context.Background(), tc.args.cr)
+
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("r: -want, +got:\n%s", diff)
+			}
+			if diff := cmp.Diff(tc.want.cr, tc.args.cr, test.EquateConditions()); diff != "" {
+				t.Errorf("r: -want, +got:\n%s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/controller/ec2/vpcendpointserviceconfiguration/setup.go b/pkg/controller/ec2/vpcendpointserviceconfiguration/setup.go
@@ -225,6 +225,9 @@ func (t *tagger) Initialize(ctx context.Context, mgd cpresource.Managed) error {
 	}
 	var vpcEndpointTags svcapitypes.TagSpecification
 	for _, tagSpecification := range cr.Spec.ForProvider.TagSpecifications {
+		if tagSpecification == nil {
+			continue
+		}
 		if aws.StringValue(tagSpecification.ResourceType) == "vpc-endpoint-service" {
 			vpcEndpointTags = *tagSpecification
 		}