Merge branch 'main' into fix-network-nil-pointer-exception

.github/workflows/lint.yml

@@ -14,13 +14,13 @@ jobs:
     steps:
       - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
 
-      - uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
+      - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2
         with:
           go-version: "1.21"
           check-latest: true
           cache: false
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1
+        uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # v6.1.0
         with:
           version: v1.58

Makefile

@@ -489,7 +489,7 @@ create-management-cluster: $(KUSTOMIZE) $(ENVSUBST) $(KIND) $(KUBECTL)
 	./hack/install-cert-manager.sh $(CERT_MANAGER_VER)
 
 	# Deploy CAPI
-	curl --retry $(CURL_RETRIES) -sSL https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.7.2/cluster-api-components.yaml | $(ENVSUBST) | $(KUBECTL) apply -f -
+	curl --retry $(CURL_RETRIES) -sSL https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.7.3/cluster-api-components.yaml | $(ENVSUBST) | $(KUBECTL) apply -f -
 
 	# Deploy CAPG
 	$(KIND) load docker-image $(CONTROLLER_IMG)-$(ARCH):$(TAG) --name=clusterapi

Tiltfile

@@ -18,7 +18,7 @@ settings = {
     "deploy_cert_manager": True,
     "preload_images_for_kind": True,
     "kind_cluster_name": "capg",
-    "capi_version": "v1.7.2",
+    "capi_version": "v1.7.3",
     "cert_manager_version": "v1.14.4",
     "kubernetes_version": "v1.29.3",
 }

api/v1beta1/gcpcluster_webhook.go

@@ -92,6 +92,20 @@ func (c *GCPCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings,
 		)
 	}
 
+	if c.Spec.Network.Mtu < int64(1300) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "Network", "Mtu"),
+				c.Spec.Network.Mtu, "field cannot be lesser than 1300"),
+		)
+	}
+
+	if c.Spec.Network.Mtu > int64(8896) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "Network", "Mtu"),
+				c.Spec.Network.Mtu, "field cannot be greater than 8896"),
+		)
+	}
+
 	if len(allErrs) == 0 {
 		return nil, nil
 	}

api/v1beta1/gcpcluster_webhook_test.go

@@ -0,0 +1,102 @@
+/*
+Copyright 2021 The Kubernetes 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 v1beta1
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestGCPCluster_ValidateUpdate(t *testing.T) {
+	g := NewWithT(t)
+
+	tests := []struct {
+		name       string
+		newCluster *GCPCluster
+		oldCluster *GCPCluster
+		wantErr    bool
+	}{
+		{
+			name: "GCPCluster with MTU field is within the limits of more than 1300 and less than 8896",
+			newCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(1500),
+					},
+				},
+			},
+			oldCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(1400),
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "GCPCluster with MTU field more than 8896",
+			newCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(10000),
+					},
+				},
+			},
+			oldCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(1500),
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "GCPCluster with MTU field less than 8896",
+			newCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(1250),
+					},
+				},
+			},
+			oldCluster: &GCPCluster{
+				Spec: GCPClusterSpec{
+					Network: NetworkSpec{
+						Mtu: int64(1500),
+					},
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			warn, err := test.newCluster.ValidateUpdate(test.oldCluster)
+			if test.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+			g.Expect(warn).To(BeNil())
+		})
+	}
+}

api/v1beta1/types.go

@@ -136,6 +136,17 @@ type NetworkSpec struct {
 	// HostProject is the name of the project hosting the shared VPC network resources.
 	// +optional
 	HostProject *string `json:"hostProject,omitempty"`
+
+	// Mtu: Maximum Transmission Unit in bytes. The minimum value for this field is
+	// 1300 and the maximum value is 8896. The suggested value is 1500, which is
+	// the default MTU used on the Internet, or 8896 if you want to use Jumbo
+	// frames. If unspecified, the value defaults to 1460.
+	// More info: https://pkg.go.dev/google.golang.org/api/compute/v1#Network
+	// +kubebuilder:validation:Minimum:=1300
+	// +kubebuilder:validation:Maximum:=8896
+	// +kubebuilder:default:=1460
+	// +optional
+	Mtu int64 `json:"mtu,omitempty"`
 }
 
 // LoadBalancerType defines the Load Balancer that should be created.

cloud/scope/cluster.go

@@ -131,6 +131,21 @@ func (s *ClusterScope) NetworkName() string {
 	return ptr.Deref(s.GCPCluster.Spec.Network.Name, "default")
 }
 
+// NetworkMtu returns the Network MTU of 1440 which is the default, otherwise returns back what is being set.
+// Mtu: Maximum Transmission Unit in bytes. The minimum value for this field is
+// 1300 and the maximum value is 8896. The suggested value is 1500, which is
+// the default MTU used on the Internet, or 8896 if you want to use Jumbo
+// frames. If unspecified, the value defaults to 1460.
+// More info
+// - https://pkg.go.dev/google.golang.org/api/compute/v1#Network
+// - https://cloud.google.com/vpc/docs/mtu
+func (s *ClusterScope) NetworkMtu() int64 {
+	if s.GCPCluster.Spec.Network.Mtu == 0 {
+		return int64(1460)
+	}
+	return s.GCPCluster.Spec.Network.Mtu
+}
+
 // NetworkLink returns the partial URL for the network.
 func (s *ClusterScope) NetworkLink() string {
 	return fmt.Sprintf("projects/%s/global/networks/%s", s.NetworkProject(), s.NetworkName())
@@ -206,6 +221,7 @@ func (s *ClusterScope) NetworkSpec() *compute.Network {
 		Description:           infrav1.ClusterTagKey(s.Name()),
 		AutoCreateSubnetworks: createSubnet,
 		ForceSendFields:       []string{"AutoCreateSubnetworks"},
+		Mtu:                   s.NetworkMtu(),
 	}
 
 	return network
@@ -337,6 +353,7 @@ func (s *ClusterScope) ForwardingRuleSpec(lbname string) *compute.ForwardingRule
 		IPProtocol:          "TCP",
 		LoadBalancingScheme: "EXTERNAL",
 		PortRange:           portRange,
+		Labels:              s.AdditionalLabels(),
 	}
 }
 

cloud/scope/machine.go

@@ -249,6 +249,7 @@ func (m *MachineScope) InstanceImageSpec() *compute.AttachedDisk {
 			DiskType:            path.Join("zones", m.Zone(), "diskTypes", string(diskType)),
 			ResourceManagerTags: shared.ResourceTagConvert(context.TODO(), m.GCPMachine.Spec.ResourceManagerTags),
 			SourceImage:         sourceImage,
+			Labels:              m.ClusterGetter.AdditionalLabels().AddLabels(m.GCPMachine.Spec.AdditionalLabels),
 		},
 	}
 

cloud/services/compute/instances/reconcile_test.go

@@ -237,6 +237,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -302,6 +305,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -369,6 +375,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -436,6 +445,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -506,6 +518,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -569,6 +584,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-a/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 					},
 				},
@@ -639,6 +657,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 						DiskEncryptionKey: &compute.CustomerEncryptionKey{
 							KmsKeyName: "projects/my-project/locations/us-central1/keyRings/us-central1/cryptoKeys/some-key",
@@ -712,6 +733,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 						DiskEncryptionKey: &compute.CustomerEncryptionKey{
 							RawKey: "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=",
@@ -785,6 +809,9 @@ func TestService_createOrGetInstance(t *testing.T) {
 							DiskType:            "zones/us-central1-c/diskTypes/pd-standard",
 							SourceImage:         "projects/my-proj/global/images/family/capi-ubuntu-1804-k8s-v1-19",
 							ResourceManagerTags: map[string]string{},
+							Labels: map[string]string{
+								"foo": "bar",
+							},
 						},
 						DiskEncryptionKey: &compute.CustomerEncryptionKey{
 							RsaEncryptedKey: "ieCx/NcW06PcT7Ep1X6LUTc/hLvUDYyzSZPPVCVPTVEohpeHASqC8uw5TzyO9U+Fka9JFHiz0mBibXUInrC/jEk014kCK/NPjYgEMOyssZ4ZINPKxlUh2zn1bV+MCaTICrdmuSBTWlUUiFoDiD6PYznLwh8ZNdaheCeZ8ewEXgFQ8V+sDroLaN3Xs3MDTXQEMMoNUXMCZEIpg9Vtp9x2oe==",

cloud/services/compute/loadbalancers/reconcile.go

@@ -579,6 +579,18 @@ func (s *Service) createOrGetForwardingRule(ctx context.Context, lbname string,
 		}
 	}
 
+	// Labels on ForwardingRules must be added after resource is created
+	labels := s.scope.AdditionalLabels()
+	if !labels.Equals(forwarding.Labels) {
+		setLabelsRequest := &compute.GlobalSetLabelsRequest{
+			LabelFingerprint: forwarding.LabelFingerprint,
+			Labels:           labels,
+		}
+		if err = s.forwardingrules.SetLabels(ctx, key, setLabelsRequest); err != nil {
+			return nil, err
+		}
+	}
+
 	return forwarding, nil
 }
 
@@ -589,9 +601,14 @@ func (s *Service) createOrGetRegionalForwardingRule(ctx context.Context, lbname
 	spec.LoadBalancingScheme = string(loadBalanceTrafficInternal)
 	spec.Region = s.scope.Region()
 	spec.BackendService = backendSvc.SelfLink
-	// Ports are used instead or PortRange for passthrough Load Balancer
-	// Configure ports for k8s API and ignition
-	spec.Ports = []string{"6443", "22623"}
+	// Ports is used instead or PortRange for passthrough Load Balancer
+	// Configure ports for k8s API to match the external API which is the first port of range
+	var ports []string
+	portList := strings.Split(spec.PortRange, "-")
+	ports = append(ports, portList[0])
+	// Also configure ignition port
+	ports = append(ports, "22623")
+	spec.Ports = ports
 	spec.PortRange = ""
 	subnet, err := s.getSubnet(ctx)
 	if err != nil {
@@ -622,6 +639,18 @@ func (s *Service) createOrGetRegionalForwardingRule(ctx context.Context, lbname
 		}
 	}
 
+	// Labels on ForwardingRules must be added after resource is created
+	labels := s.scope.AdditionalLabels()
+	if !labels.Equals(forwarding.Labels) {
+		setLabelsRequest := &compute.RegionSetLabelsRequest{
+			LabelFingerprint: forwarding.LabelFingerprint,
+			Labels:           labels,
+		}
+		if err = s.regionalforwardingrules.SetLabels(ctx, key, setLabelsRequest); err != nil {
+			return nil, err
+		}
+	}
+
 	return forwarding, nil
 }