From f257382ac5a36595cd3436509f5e07981339e6ad Mon Sep 17 00:00:00 2001 From: Brent Barbachem Date: Fri, 22 Mar 2024 12:23:37 -0400 Subject: [PATCH 1/2] Add support for Shared VPC Networking This is an update to the stale PR #991 ** Added support for Host Project for a shared VPC in the Network struct. ** Network Resources will now use the host project name if exists, otherwise the normal project. ** Update the cluster getter interface to include the NetworkProject and Indicator for a shared VPC. ** Update reconcilers for girewall rules, subnets and network. ** Update the services to use the host project for resources when a shared vpc is used. --- api/v1beta1/types.go | 4 ++++ api/v1beta1/zz_generated.deepcopy.go | 5 +++++ cloud/interfaces.go | 3 +++ cloud/scope/cluster.go | 18 +++++++++++++++++- cloud/scope/machine.go | 9 +++++++-- cloud/scope/managedcluster.go | 18 +++++++++++++++++- cloud/services/compute/firewalls/reconcile.go | 8 ++++++++ cloud/services/compute/networks/reconcile.go | 16 ++++++++++++++++ cloud/services/compute/networks/service.go | 9 +++++++-- cloud/services/compute/subnets/reconcile.go | 9 +++++++++ cloud/services/compute/subnets/service.go | 7 ++++++- ...structure.cluster.x-k8s.io_gcpclusters.yaml | 4 ++++ ...e.cluster.x-k8s.io_gcpclustertemplates.yaml | 4 ++++ ...re.cluster.x-k8s.io_gcpmanagedclusters.yaml | 4 ++++ 14 files changed, 111 insertions(+), 7 deletions(-) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 763916cb2..50c5734c8 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -112,6 +112,10 @@ type NetworkSpec struct { // Allow for configuration of load balancer backend (useful for changing apiserver port) // +optional LoadBalancerBackendPort *int32 `json:"loadBalancerBackendPort,omitempty"` + + // HostProject is the name of the project hosting the shared VPC network resources. + // +optional + HostProject *string `json:"hostProject,omitempty"` } // LoadBalancerSpec contains configuration for one or more LoadBalancers. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index d3e20f307..c4b30ff4d 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -818,6 +818,11 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = new(int32) **out = **in } + if in.HostProject != nil { + in, out := &in.HostProject, &out.HostProject + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkSpec. diff --git a/cloud/interfaces.go b/cloud/interfaces.go index 17e627cb4..e2652a3c0 100644 --- a/cloud/interfaces.go +++ b/cloud/interfaces.go @@ -46,6 +46,7 @@ type ReconcilerWithResult interface { // Client is an interface which can get cloud client. type Client interface { Cloud() Cloud + NetworkCloud() Cloud } // ClusterGetter is an interface which can get cluster information. @@ -56,6 +57,8 @@ type ClusterGetter interface { Name() string Namespace() string NetworkName() string + NetworkProject() string + IsSharedVpc() bool Network() *infrav1.Network AdditionalLabels() infrav1.Labels FailureDomains() clusterv1.FailureDomains diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 6f7cb0a98..a4ebcb9ee 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -90,11 +90,27 @@ func (s *ClusterScope) Cloud() cloud.Cloud { return newCloud(s.Project(), s.GCPServices) } +// NetworkCloud returns initialized cloud. +func (s *ClusterScope) NetworkCloud() cloud.Cloud { + return newCloud(s.NetworkProject(), s.GCPServices) +} + // Project returns the current project name. func (s *ClusterScope) Project() string { return s.GCPCluster.Spec.Project } +// NetworkProject returns the project name where network resources should exist. +// The network project defaults to the Project when one is not supplied. +func (s *ClusterScope) NetworkProject() string { + return ptr.Deref(s.GCPCluster.Spec.Network.HostProject, s.Project()) +} + +// IsSharedVpc returns true If sharedVPC used else , returns false. +func (s *ClusterScope) IsSharedVpc() bool { + return s.NetworkProject() != s.Project() +} + // Region returns the cluster region. func (s *ClusterScope) Region() string { return s.GCPCluster.Spec.Region @@ -117,7 +133,7 @@ func (s *ClusterScope) NetworkName() string { // NetworkLink returns the partial URL for the network. func (s *ClusterScope) NetworkLink() string { - return fmt.Sprintf("projects/%s/global/networks/%s", s.Project(), s.NetworkName()) + return fmt.Sprintf("projects/%s/global/networks/%s", s.NetworkProject(), s.NetworkName()) } // Network returns the cluster network object. diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 918b98ee1..695160bd1 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -94,6 +94,11 @@ func (m *MachineScope) Cloud() cloud.Cloud { return m.ClusterGetter.Cloud() } +// NetworkCloud returns initialized network cloud. +func (m *MachineScope) NetworkCloud() cloud.Cloud { + return m.ClusterGetter.NetworkCloud() +} + // Zone returns the FailureDomain for the GCPMachine. func (m *MachineScope) Zone() string { if m.Machine.Spec.FailureDomain == nil { @@ -319,7 +324,7 @@ func (m *MachineScope) InstanceAdditionalDiskSpec() []*compute.AttachedDisk { // InstanceNetworkInterfaceSpec returns compute network interface spec. func (m *MachineScope) InstanceNetworkInterfaceSpec() *compute.NetworkInterface { networkInterface := &compute.NetworkInterface{ - Network: path.Join("projects", m.ClusterGetter.Project(), "global", "networks", m.ClusterGetter.NetworkName()), + Network: path.Join("projects", m.ClusterGetter.NetworkProject(), "global", "networks", m.ClusterGetter.NetworkName()), } if m.GCPMachine.Spec.PublicIP != nil && *m.GCPMachine.Spec.PublicIP { @@ -332,7 +337,7 @@ func (m *MachineScope) InstanceNetworkInterfaceSpec() *compute.NetworkInterface } if m.GCPMachine.Spec.Subnet != nil { - networkInterface.Subnetwork = path.Join("regions", m.ClusterGetter.Region(), "subnetworks", *m.GCPMachine.Spec.Subnet) + networkInterface.Subnetwork = path.Join("projects", m.ClusterGetter.NetworkProject(), "regions", m.ClusterGetter.Region(), "subnetworks", *m.GCPMachine.Spec.Subnet) } return networkInterface diff --git a/cloud/scope/managedcluster.go b/cloud/scope/managedcluster.go index e9445a4be..87f87dff0 100644 --- a/cloud/scope/managedcluster.go +++ b/cloud/scope/managedcluster.go @@ -93,6 +93,11 @@ func (s *ManagedClusterScope) Cloud() cloud.Cloud { return newCloud(s.Project(), s.GCPServices) } +// NetworkCloud returns initialized cloud. +func (s *ManagedClusterScope) NetworkCloud() cloud.Cloud { + return newCloud(s.NetworkProject(), s.GCPServices) +} + // Project returns the current project name. func (s *ManagedClusterScope) Project() string { return s.GCPManagedCluster.Spec.Project @@ -118,9 +123,20 @@ func (s *ManagedClusterScope) NetworkName() string { return ptr.Deref(s.GCPManagedCluster.Spec.Network.Name, "default") } +// NetworkProject returns the project name where network resources should exist. +// The network project defaults to the Project when one is not supplied. +func (s *ManagedClusterScope) NetworkProject() string { + return ptr.Deref(s.GCPManagedCluster.Spec.Network.HostProject, s.Project()) +} + +// IsSharedVpc returns true If sharedVPC used else , returns false. +func (s *ManagedClusterScope) IsSharedVpc() bool { + return s.NetworkProject() != s.Project() +} + // NetworkLink returns the partial URL for the network. func (s *ManagedClusterScope) NetworkLink() string { - return fmt.Sprintf("projects/%s/global/networks/%s", s.Project(), s.NetworkName()) + return fmt.Sprintf("projects/%s/global/networks/%s", s.NetworkProject(), s.NetworkName()) } // Network returns the cluster network object. diff --git a/cloud/services/compute/firewalls/reconcile.go b/cloud/services/compute/firewalls/reconcile.go index d76add182..4047f6548 100644 --- a/cloud/services/compute/firewalls/reconcile.go +++ b/cloud/services/compute/firewalls/reconcile.go @@ -28,6 +28,10 @@ import ( // Reconcile reconcile cluster firewall compoenents. func (s *Service) Reconcile(ctx context.Context) error { log := log.FromContext(ctx) + if s.scope.IsSharedVpc() { + log.V(2).Info("Shared VPC enabled. Ignore Reconciling firewall resources") + return nil + } log.Info("Reconciling firewall resources") for _, spec := range s.scope.FirewallRulesSpec() { log.V(2).Info("Looking firewall", "name", spec.Name) @@ -50,6 +54,10 @@ func (s *Service) Reconcile(ctx context.Context) error { // Delete delete cluster firewall compoenents. func (s *Service) Delete(ctx context.Context) error { log := log.FromContext(ctx) + if s.scope.IsSharedVpc() { + log.V(2).Info("Shared VPC enabled. Ignore Deleting firewall resources") + return nil + } log.Info("Deleting firewall resources") for _, spec := range s.scope.FirewallRulesSpec() { log.V(2).Info("Deleting firewall", "name", spec.Name) diff --git a/cloud/services/compute/networks/reconcile.go b/cloud/services/compute/networks/reconcile.go index 192c2ccfc..daf7ae987 100644 --- a/cloud/services/compute/networks/reconcile.go +++ b/cloud/services/compute/networks/reconcile.go @@ -54,6 +54,12 @@ func (s *Service) Reconcile(ctx context.Context) error { // Delete delete cluster network components. func (s *Service) Delete(ctx context.Context) error { log := log.FromContext(ctx) + if s.scope.IsSharedVpc() { + log.V(2).Info("Shared VPC enabled. Ignore Deleting network resources") + s.scope.Network().Router = nil + s.scope.Network().SelfLink = nil + return nil + } log.Info("Deleting network resources") networkKey := meta.GlobalKey(s.scope.NetworkName()) log.V(2).Info("Looking for network before deleting", "name", networkKey) @@ -104,6 +110,11 @@ func (s *Service) createOrGetNetwork(ctx context.Context) (*compute.Network, err return nil, err } + if s.scope.IsSharedVpc() { + log.Error(err, "Shared VPC is enabled, but could not find existing network", "name", s.scope.NetworkName()) + return nil, err + } + log.V(2).Info("Creating a network", "name", s.scope.NetworkName()) if err := s.networks.Insert(ctx, networkKey, s.scope.NetworkSpec()); err != nil { log.Error(err, "Error creating a network", "name", s.scope.NetworkName()) @@ -132,6 +143,11 @@ func (s *Service) createOrGetRouter(ctx context.Context, network *compute.Networ return nil, err } + if s.scope.IsSharedVpc() { + log.Error(err, "Shared VPC is enabled, but could not find existing router", "name", routerKey) + return nil, err + } + spec.Network = network.SelfLink spec.Description = infrav1.ClusterTagKey(s.scope.Name()) log.V(2).Info("Creating a cloudnat router", "name", spec.Name) diff --git a/cloud/services/compute/networks/service.go b/cloud/services/compute/networks/service.go index a21eabf92..cf0430e09 100644 --- a/cloud/services/compute/networks/service.go +++ b/cloud/services/compute/networks/service.go @@ -56,9 +56,14 @@ var _ cloud.Reconciler = &Service{} // New returns Service from given scope. func New(scope Scope) *Service { + scopeCloud := scope.Cloud() + if scope.IsSharedVpc() { + scopeCloud = scope.NetworkCloud() + } + return &Service{ scope: scope, - networks: scope.Cloud().Networks(), - routers: scope.Cloud().Routers(), + networks: scopeCloud.Networks(), + routers: scopeCloud.Routers(), } } diff --git a/cloud/services/compute/subnets/reconcile.go b/cloud/services/compute/subnets/reconcile.go index cea4d1de7..52ca259d3 100644 --- a/cloud/services/compute/subnets/reconcile.go +++ b/cloud/services/compute/subnets/reconcile.go @@ -44,6 +44,10 @@ func (s *Service) Reconcile(ctx context.Context) error { // Delete deletes cluster subnetwork components. func (s *Service) Delete(ctx context.Context) error { logger := log.FromContext(ctx) + if s.scope.IsSharedVpc() { + logger.V(2).Info("Shared VPC enabled. Skip deleting subnet resources") + return nil + } for _, subnetSpec := range s.scope.SubnetSpecs() { subnetKey := meta.RegionalKey(subnetSpec.Name, s.getSubnetRegion(subnetSpec)) logger.V(2).Info("Looking for subnet before deleting it", "name", subnetSpec.Name) @@ -89,6 +93,11 @@ func (s *Service) createOrGetSubnets(ctx context.Context) ([]*compute.Subnetwork return subnets, err } + if s.scope.IsSharedVpc() { + logger.Error(err, "Shared VPC is enabled, but could not find existing subnetwork", "name", subnetSpec.Name) + return nil, err + } + // Subnet was not found, let's create it logger.V(2).Info("Creating a subnet", "name", subnetSpec.Name) if err := s.subnets.Insert(ctx, subnetKey, subnetSpec); err != nil { diff --git a/cloud/services/compute/subnets/service.go b/cloud/services/compute/subnets/service.go index 565442d20..d3ad65db8 100644 --- a/cloud/services/compute/subnets/service.go +++ b/cloud/services/compute/subnets/service.go @@ -48,8 +48,13 @@ var _ cloud.Reconciler = &Service{} // New returns Service from given scope. func New(scope Scope) *Service { + cloudScope := scope.Cloud() + if scope.IsSharedVpc() { + cloudScope = scope.NetworkCloud() + } + return &Service{ scope: scope, - subnets: scope.Cloud().Subnetworks(), + subnets: cloudScope.Subnetworks(), } } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml index 10fe9b1dc..abcf3a364 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml @@ -136,6 +136,10 @@ spec: Defaults to true. type: boolean + hostProject: + description: HostProject is the name of the project hosting the + shared VPC network resources. + type: string loadBalancerBackendPort: description: Allow for configuration of load balancer backend (useful for changing apiserver port) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml index 05179c5c3..f36deb5c0 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml @@ -153,6 +153,10 @@ spec: Defaults to true. type: boolean + hostProject: + description: HostProject is the name of the project hosting + the shared VPC network resources. + type: string loadBalancerBackendPort: description: Allow for configuration of load balancer backend (useful for changing apiserver port) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml index 7faef1da9..e7b7189e1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml @@ -132,6 +132,10 @@ spec: Defaults to true. type: boolean + hostProject: + description: HostProject is the name of the project hosting the + shared VPC network resources. + type: string loadBalancerBackendPort: description: Allow for configuration of load balancer backend (useful for changing apiserver port) From cd4236ffcaa849c465341b8661bd08ae86b9e5d5 Mon Sep 17 00:00:00 2001 From: Brent Barbachem Date: Tue, 26 Mar 2024 16:41:28 -0400 Subject: [PATCH 2/2] ** Added tests for compute -> firewalls, subnets, network --- .../compute/firewalls/reconcile_test.go | 310 ++++++++++++++ .../compute/networks/reconcile_test.go | 403 ++++++++++++++++++ .../compute/subnets/reconcile_test.go | 70 ++- 3 files changed, 782 insertions(+), 1 deletion(-) create mode 100644 cloud/services/compute/firewalls/reconcile_test.go create mode 100644 cloud/services/compute/networks/reconcile_test.go diff --git a/cloud/services/compute/firewalls/reconcile_test.go b/cloud/services/compute/firewalls/reconcile_test.go new file mode 100644 index 000000000..cedc2375f --- /dev/null +++ b/cloud/services/compute/firewalls/reconcile_test.go @@ -0,0 +1,310 @@ +/* +Copyright 2024 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 firewalls + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" + "github.com/pkg/errors" + "google.golang.org/api/compute/v1" + "google.golang.org/api/googleapi" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-gcp/cloud/scope" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func init() { + _ = clusterv1.AddToScheme(scheme.Scheme) + _ = infrav1.AddToScheme(scheme.Scheme) +} + +var fakeCluster = &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: clusterv1.ClusterSpec{}, +} + +var fakeGCPCluster = &infrav1.GCPCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1.GCPClusterSpec{ + Project: "my-proj", + Region: "us-central1", + Network: infrav1.NetworkSpec{ + Name: ptr.To("my-network"), + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + Name: "workers", + CidrBlock: "10.0.0.1/28", + Region: "us-central1", + Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"), + }, + }, + }, + }, + Status: infrav1.GCPClusterStatus{ + Network: infrav1.Network{ + FirewallRules: map[string]string{ + fmt.Sprintf("allow-%s-healthchecks", "my-cluster"): "test", + }, + }, + }, +} + +var fakeGCPClusterSharedVPC = &infrav1.GCPCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1.GCPClusterSpec{ + Project: "my-proj", + Region: "us-central1", + Network: infrav1.NetworkSpec{ + HostProject: ptr.To("my-shared-vpc-project"), + Name: ptr.To("my-network"), + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + Name: "workers", + CidrBlock: "10.0.0.1/28", + Region: "us-central1", + Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"), + }, + }, + }, + }, + Status: infrav1.GCPClusterStatus{ + Network: infrav1.Network{ + FirewallRules: map[string]string{ + "my-cluster-apiserver": "test", + "my-cluster-apiintserver": "test", + }, + }, + }, +} + +type testCase struct { + name string + scope func() Scope + mockFirewalls *cloud.MockFirewalls + wantErr bool + assert func(ctx context.Context, t testCase) error +} + +func TestService_Reconcile(t *testing.T) { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPCluster, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + tests := []testCase{ + { + name: "firewall rule does not exist successful create", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{}, + }, + assert: func(ctx context.Context, t testCase) error { + key := meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)) + fwRule, err := t.mockFirewalls.Get(ctx, key) + if err != nil { + return err + } + + if _, ok := fakeGCPCluster.Status.Network.FirewallRules[fwRule.Name]; !ok { + return errors.New("firewall rule was created but with wrong values") + } + return nil + }, + }, + { + name: "firewall rule already exist (should return existing firewall rule)", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): {}, + }, + }, + }, + { + name: "error getting instance with non 404 error code (should return an error)", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{}, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockFirewalls, _ ...cloud.Option) (bool, *compute.Firewall, error) { + return true, &compute.Firewall{}, &googleapi.Error{Code: http.StatusBadRequest} + }, + }, + wantErr: true, + }, + { + name: "firewall rule creation fails (should return an error)", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{}, + InsertError: map[meta.Key]error{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusBadRequest}, + }, + }, + wantErr: true, + }, + { + name: "firewall return no error using shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): {}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + s := New(tt.scope()) + s.firewalls = tt.mockFirewalls + err := s.Reconcile(ctx) + if (err != nil) != tt.wantErr { + t.Errorf("Service.Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.assert != nil { + err = tt.assert(ctx, tt) + if err != nil { + t.Errorf("firewall rule was not created as expected: %v", err) + return + } + } + }) + } +} + +func TestService_Delete(t *testing.T) { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPCluster, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + tests := []testCase{ + { + name: "firewall rule does not exist, should do nothing", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + DeleteError: map[meta.Key]error{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusNotFound}, + }, + }, + }, + { + name: "error deleting firewall rule, should return error", + scope: func() Scope { return clusterScope }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + DeleteError: map[meta.Key]error{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.ObjectMeta.Name)): &googleapi.Error{Code: http.StatusBadRequest}, + }, + }, + wantErr: true, + }, + { + name: "firewall rule deletion with shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + DeleteError: map[meta.Key]error{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", *fakeGCPCluster.Spec.Network.Name)): &googleapi.Error{Code: http.StatusNotFound}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + s := New(tt.scope()) + s.firewalls = tt.mockFirewalls + err := s.Delete(ctx) + if (err != nil) != tt.wantErr { + t.Errorf("Service.Delete() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} diff --git a/cloud/services/compute/networks/reconcile_test.go b/cloud/services/compute/networks/reconcile_test.go new file mode 100644 index 000000000..002fac586 --- /dev/null +++ b/cloud/services/compute/networks/reconcile_test.go @@ -0,0 +1,403 @@ +/* +Copyright 2024 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 networks + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" + "google.golang.org/api/compute/v1" + "google.golang.org/api/googleapi" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" + infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-gcp/cloud/scope" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func init() { + _ = clusterv1.AddToScheme(scheme.Scheme) + _ = infrav1.AddToScheme(scheme.Scheme) +} + +var fakeCluster = &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: clusterv1.ClusterSpec{}, +} + +var fakeGCPCluster = &infrav1.GCPCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1.GCPClusterSpec{ + Project: "my-proj", + Region: "us-central1", + Network: infrav1.NetworkSpec{ + Name: ptr.To("my-network"), + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + Name: "workers", + CidrBlock: "10.0.0.1/28", + Region: "us-central1", + Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"), + }, + }, + }, + }, +} + +var fakeGCPClusterSharedVPC = &infrav1.GCPCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1.GCPClusterSpec{ + Project: "my-proj", + Region: "us-central1", + Network: infrav1.NetworkSpec{ + HostProject: ptr.To("my-shared-vpc-project"), + Name: ptr.To("my-network"), + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + Name: "workers", + CidrBlock: "10.0.0.1/28", + Region: "us-central1", + Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"), + }, + }, + }, + }, +} + +type testCase struct { + name string + scope func() Scope + mockNetwork *cloud.MockNetworks + mockRouter *cloud.MockRouters + wantErr bool + assert func(ctx context.Context, t testCase) error +} + +func TestService_createOrGetNetwork(t *testing.T) { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPCluster, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + tests := []testCase{ + { + name: "network already exist (should return existing network)", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + }, + }, + { + name: "error getting network instance with non 404 error code (should return an error)", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockNetworks, _ ...cloud.Option) (bool, *compute.Network, error) { + return true, &compute.Network{}, &googleapi.Error{Code: http.StatusBadRequest} + }, + }, + wantErr: true, + }, + { + name: "network list error find issue shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockNetworks, _ ...cloud.Option) (bool, *compute.Network, error) { + return true, &compute.Network{}, &googleapi.Error{Code: http.StatusNotFound} + }, + }, + wantErr: true, + }, + { + name: "network creation fails (should return an error)", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{}, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockNetworks, _ ...cloud.Option) (bool, *compute.Network, error) { + return true, &compute.Network{}, &googleapi.Error{Code: http.StatusNotFound} + }, + InsertError: map[meta.Key]error{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): &googleapi.Error{Code: http.StatusBadRequest}, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + s := New(tt.scope()) + s.networks = tt.mockNetwork + _, err := s.createOrGetNetwork(ctx) + if (err != nil) != tt.wantErr { + t.Errorf("Service.createOrGetNetwork error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.assert != nil { + err = tt.assert(ctx, tt) + if err != nil { + t.Errorf("network was not created as expected: %v", err) + return + } + } + }) + } +} + +func TestService_createOrGetRouter(t *testing.T) { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPCluster, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + tests := []testCase{ + { + name: "error getting router instance with non 404 error code (should return an error)", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + }, + mockRouter: &cloud.MockRouters{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockRoutersObj{ + *meta.RegionalKey(fmt.Sprintf("%s-%s", *fakeGCPCluster.Spec.Network.Name, "router"), fakeGCPCluster.Spec.Region): {}, + }, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockRouters, _ ...cloud.Option) (bool, *compute.Router, error) { + return true, &compute.Router{}, &googleapi.Error{Code: http.StatusBadRequest} + }, + }, + wantErr: true, + }, + { + name: "router list error find issue shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + }, + mockRouter: &cloud.MockRouters{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockRoutersObj{ + *meta.RegionalKey(fmt.Sprintf("%s-%s", *fakeGCPCluster.Spec.Network.Name, "router"), fakeGCPCluster.Spec.Region): {}, + }, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockRouters, _ ...cloud.Option) (bool, *compute.Router, error) { + return true, &compute.Router{}, &googleapi.Error{Code: http.StatusBadRequest} + }, + }, + wantErr: true, + }, + { + name: "router creation fails (should return an error)", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockNetworksObj{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): {}, + }, + }, + mockRouter: &cloud.MockRouters{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockRoutersObj{}, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockRouters, _ ...cloud.Option) (bool, *compute.Router, error) { + return true, &compute.Router{}, &googleapi.Error{Code: http.StatusNotFound} + }, + InsertError: map[meta.Key]error{ + *meta.RegionalKey(fmt.Sprintf("%s-%s", *fakeGCPCluster.Spec.Network.Name, "router"), fakeGCPCluster.Spec.Region): &googleapi.Error{Code: http.StatusBadRequest}, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + s := New(tt.scope()) + s.networks = tt.mockNetwork + s.routers = tt.mockRouter + + network, err := s.createOrGetNetwork(ctx) + if err != nil { + t.Errorf("Service.createOrGetNetwork error = %v", err) + return + } + + _, err = s.createOrGetRouter(ctx, network) + if (err != nil) != tt.wantErr { + t.Errorf("Service.createOrGetRouter error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.assert != nil { + err = tt.assert(ctx, tt) + if err != nil { + t.Errorf("router was not created as expected: %v", err) + return + } + } + }) + } +} + +func TestService_Delete(t *testing.T) { + fakec := fake.NewClientBuilder(). + WithScheme(scheme.Scheme). + Build() + + clusterScope, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPCluster, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + + tests := []testCase{ + { + name: "network does not exist, should do nothing", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + GetError: map[meta.Key]error{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): &googleapi.Error{Code: http.StatusNotFound}, + }, + }, + }, + { + name: "error deleting network, should return error", + scope: func() Scope { return clusterScope }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + GetError: map[meta.Key]error{ + *meta.GlobalKey(*fakeGCPCluster.Spec.Network.Name): &googleapi.Error{Code: http.StatusBadGateway}, + }, + }, + wantErr: true, + }, + { + name: "network shared vpc, should do nothing", + scope: func() Scope { return clusterScopeSharedVpc }, + mockNetwork: &cloud.MockNetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + s := New(tt.scope()) + s.networks = tt.mockNetwork + err := s.Delete(ctx) + if (err != nil) != tt.wantErr { + t.Errorf("Service.Delete() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} diff --git a/cloud/services/compute/subnets/reconcile_test.go b/cloud/services/compute/subnets/reconcile_test.go index ebc23aa68..e3015361d 100644 --- a/cloud/services/compute/subnets/reconcile_test.go +++ b/cloud/services/compute/subnets/reconcile_test.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2024 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. @@ -72,6 +72,28 @@ var fakeGCPCluster = &infrav1.GCPCluster{ }, } +var fakeGCPClusterSharedVPC = &infrav1.GCPCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + Namespace: "default", + }, + Spec: infrav1.GCPClusterSpec{ + Project: "my-proj", + Region: "us-central1", + Network: infrav1.NetworkSpec{ + HostProject: ptr.To("my-shared-vpc-project"), + Subnets: infrav1.Subnets{ + infrav1.SubnetSpec{ + Name: "workers", + CidrBlock: "10.0.0.1/28", + Region: "us-central1", + Purpose: ptr.To[string]("INTERNAL_HTTPS_LOAD_BALANCER"), + }, + }, + }, + }, +} + type testCase struct { name string scope func() Scope @@ -97,6 +119,18 @@ func TestService_Reconcile(t *testing.T) { t.Fatal(err) } + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + tests := []testCase{ { name: "subnet already exist (should return existing subnet)", @@ -155,6 +189,18 @@ func TestService_Reconcile(t *testing.T) { }, wantErr: true, }, + { + name: "subnet list error find issue shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockSubnetworks: &cloud.MockSubnetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockSubnetworksObj{}, + GetHook: func(_ context.Context, _ *meta.Key, _ *cloud.MockSubnetworks, _ ...cloud.Option) (bool, *compute.Subnetwork, error) { + return true, &compute.Subnetwork{}, &googleapi.Error{Code: http.StatusBadRequest} + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -194,6 +240,18 @@ func TestService_Delete(t *testing.T) { t.Fatal(err) } + clusterScopeSharedVpc, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterSharedVPC, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + tests := []testCase{ { name: "subnet does not exist, should do nothing", @@ -233,6 +291,16 @@ func TestService_Delete(t *testing.T) { }, wantErr: false, }, + { + name: "subnet deletion with shared vpc", + scope: func() Scope { return clusterScopeSharedVpc }, + mockSubnetworks: &cloud.MockSubnetworks{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + DeleteError: map[meta.Key]error{ + *meta.RegionalKey(fakeGCPCluster.Spec.Network.Subnets[0].Name, fakeGCPCluster.Spec.Region): &googleapi.Error{Code: http.StatusNotFound}, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {