From ea29531b5becf2a44b9625789240c26cd49e5c53 Mon Sep 17 00:00:00 2001 From: barbacbd Date: Thu, 18 Sep 2025 15:00:43 -0400 Subject: [PATCH] CORS-4230: Add a firewall spec and the ability to manage or unmanaged firewall rule creation api: Add API changes to Skip firewall rule creation. When unmanaged, the firewall rules will not be created. When this is the case, the firewall rules should exist prior to creating the network. This will allow ServiceAccounts to skip the rules: compute.firewalls.create cloud: Update the services and interfaces. The firewall service will no longer create firewall rules when the firewall policy is set to unmanaged OR when a shared vpc is used during installation and resource creation. --- api/v1beta1/types.go | 34 +++++++++++++ api/v1beta1/zz_generated.deepcopy.go | 16 ++++++ cloud/interfaces.go | 1 + cloud/scope/cluster.go | 8 +++ cloud/scope/managedcluster.go | 8 +++ cloud/services/compute/firewalls/reconcile.go | 4 +- .../compute/firewalls/reconcile_test.go | 50 +++++++++++++++++++ ...tructure.cluster.x-k8s.io_gcpclusters.yaml | 20 ++++++++ ....cluster.x-k8s.io_gcpclustertemplates.yaml | 20 ++++++++ ...e.cluster.x-k8s.io_gcpmanagedclusters.yaml | 20 ++++++++ ...r.x-k8s.io_gcpmanagedclustertemplates.yaml | 20 ++++++++ 11 files changed, 199 insertions(+), 2 deletions(-) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index bbe110191..727f6e982 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -107,6 +107,36 @@ type Network struct { APIInternalForwardingRule *string `json:"apiInternalForwardingRule,omitempty"` } +// FirewallSpec contains configuration for the firewall. +type FirewallSpec struct { + // DefaultRulesManagement determines the management policy for the default firewall rules + // created by the controller. DefaultRulesManagement has no effect on user specified firewall + // rules. DefaultRulesManagement has no effect when a HostProject is specified. + // "Managed": The controller will create and manage firewall rules. + // "Unmanaged": The controller will not create or modify any firewall rules. If + // the RulesManagement is changed from Managed to Unmanaged after the firewall rules + // have been created, then the firewall rules will not be deleted. + // + // Defaults to "Managed". + // +optional + // +kubebuilder:default:="Managed" + DefaultRulesManagement RulesManagementPolicy `json:"defaultRulesManagement,omitempty"` +} + +// RulesManagementPolicy is a string enum type for managing firewall rules. +// +kubebuilder:validation:Enum=Managed;Unmanaged +type RulesManagementPolicy string + +const ( + // RulesManagementManaged indicates that the controller should create and manage + // firewall rules. This is the default behavior. + RulesManagementManaged RulesManagementPolicy = "Managed" + + // RulesManagementUnmanaged indicates that the controller should not create or manage + // any firewall rules. If rules already exist, they will be left as-is. + RulesManagementUnmanaged RulesManagementPolicy = "Unmanaged" +) + // NetworkSpec encapsulates all things related to a GCP network. type NetworkSpec struct { // Name is the name of the network to be used. @@ -137,6 +167,10 @@ type NetworkSpec struct { // +optional HostProject *string `json:"hostProject,omitempty"` + // Firewall configuration. + // +optional + Firewall FirewallSpec `json:"firewall,omitempty,omitzero"` + // 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 diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 9d31ffc4f..328442236 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -133,6 +133,21 @@ func (in *Filter) DeepCopy() *Filter { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallSpec) DeepCopyInto(out *FirewallSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallSpec. +func (in *FirewallSpec) DeepCopy() *FirewallSpec { + if in == nil { + return nil + } + out := new(FirewallSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GCPCluster) DeepCopyInto(out *GCPCluster) { *out = *in @@ -892,6 +907,7 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = new(string) **out = **in } + out.Firewall = in.Firewall } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkSpec. diff --git a/cloud/interfaces.go b/cloud/interfaces.go index 27abdcb42..c7cf79021 100644 --- a/cloud/interfaces.go +++ b/cloud/interfaces.go @@ -58,6 +58,7 @@ type ClusterGetter interface { NetworkName() string NetworkProject() string IsSharedVpc() bool + SkipFirewallRuleCreation() bool Network() *infrav1.Network AdditionalLabels() infrav1.Labels FailureDomains() clusterv1.FailureDomains diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 706bdad3b..2df32d23a 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -106,6 +106,14 @@ func (s *ClusterScope) NetworkProject() string { return ptr.Deref(s.GCPCluster.Spec.Network.HostProject, s.Project()) } +// SkipFirewallRuleCreation returns whether the spec indicates that firewall rules +// should be created or not. If the RulesManagement for the default firewall rules is +// set to unmanaged or when the cluster will include a shared VPC, the default firewall +// rule creation will be skipped. +func (s *ClusterScope) SkipFirewallRuleCreation() bool { + return (s.GCPCluster.Spec.Network.Firewall.DefaultRulesManagement == infrav1.RulesManagementUnmanaged) || s.IsSharedVpc() +} + // IsSharedVpc returns true If sharedVPC used else , returns false. func (s *ClusterScope) IsSharedVpc() bool { return s.NetworkProject() != s.Project() diff --git a/cloud/scope/managedcluster.go b/cloud/scope/managedcluster.go index 51ec04a43..e163ff007 100644 --- a/cloud/scope/managedcluster.go +++ b/cloud/scope/managedcluster.go @@ -129,6 +129,14 @@ func (s *ManagedClusterScope) NetworkProject() string { return ptr.Deref(s.GCPManagedCluster.Spec.Network.HostProject, s.Project()) } +// SkipFirewallRuleCreation returns whether the spec indicates that firewall rules +// should be created or not. If the RulesManagement for the default firewall rules is +// set to unmanaged or when the cluster will include a shared VPC, the default firewall +// rule creation will be skipped. +func (s *ManagedClusterScope) SkipFirewallRuleCreation() bool { + return (s.GCPManagedCluster.Spec.Network.Firewall.DefaultRulesManagement == infrav1.RulesManagementUnmanaged) || s.IsSharedVpc() +} + // IsSharedVpc returns true If sharedVPC used else , returns false. func (s *ManagedClusterScope) IsSharedVpc() bool { return s.NetworkProject() != s.Project() diff --git a/cloud/services/compute/firewalls/reconcile.go b/cloud/services/compute/firewalls/reconcile.go index 4047f6548..0d65a2d9d 100644 --- a/cloud/services/compute/firewalls/reconcile.go +++ b/cloud/services/compute/firewalls/reconcile.go @@ -28,8 +28,8 @@ 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") + if s.scope.SkipFirewallRuleCreation() { + log.V(2).Info("Ignore Reconciling firewall resources") return nil } log.Info("Reconciling firewall resources") diff --git a/cloud/services/compute/firewalls/reconcile_test.go b/cloud/services/compute/firewalls/reconcile_test.go index 7a927aed5..aa1b080af 100644 --- a/cloud/services/compute/firewalls/reconcile_test.go +++ b/cloud/services/compute/firewalls/reconcile_test.go @@ -109,6 +109,34 @@ var fakeGCPClusterSharedVPC = &infrav1.GCPCluster{ }, } +var fakeGCPClusterUnmanagedFirewalls = &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"), + }, + }, + Firewall: infrav1.FirewallSpec{ + DefaultRulesManagement: infrav1.RulesManagementUnmanaged, + }, + }, + }, + Status: infrav1.GCPClusterStatus{ + Network: infrav1.Network{}, + }, +} + type testCase struct { name string scope func() Scope @@ -146,6 +174,18 @@ func TestService_Reconcile(t *testing.T) { t.Fatal(err) } + clusterScopeUnmanagedFirewalls, err := scope.NewClusterScope(context.TODO(), scope.ClusterScopeParams{ + Client: fakec, + Cluster: fakeCluster, + GCPCluster: fakeGCPClusterUnmanagedFirewalls, + GCPServices: scope.GCPServices{ + Compute: &compute.Service{}, + }, + }) + if err != nil { + t.Fatal(err) + } + tests := []testCase{ { name: "firewall rule does not exist successful create", @@ -211,6 +251,16 @@ func TestService_Reconcile(t *testing.T) { }, }, }, + { + name: "firewall return no error using unmanaged firewall settings", + scope: func() Scope { return clusterScopeUnmanagedFirewalls }, + mockFirewalls: &cloud.MockFirewalls{ + ProjectRouter: &cloud.SingleProjectRouter{ID: "my-proj"}, + Objects: map[meta.Key]*cloud.MockFirewallsObj{ + *meta.GlobalKey(fmt.Sprintf("allow-%s-healthchecks", fakeGCPCluster.Name)): {}, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { 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 ea314885b..0e070be7b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml @@ -179,6 +179,26 @@ spec: Defaults to true. type: boolean + firewall: + description: Firewall configuration. + properties: + defaultRulesManagement: + default: Managed + description: |- + DefaultRulesManagement determines the management policy for the default firewall rules + created by the controller. DefaultRulesManagement has no effect on user specified firewall + rules. DefaultRulesManagement has no effect when a HostProject is specified. + "Managed": The controller will create and manage firewall rules. + "Unmanaged": The controller will not create or modify any firewall rules. If + the RulesManagement is changed from Managed to Unmanaged after the firewall rules + have been created, then the firewall rules will not be deleted. + + Defaults to "Managed". + enum: + - Managed + - Unmanaged + type: string + type: object hostProject: description: HostProject is the name of the project hosting the shared VPC network resources. 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 b55917eea..a6bfbfb87 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml @@ -198,6 +198,26 @@ spec: Defaults to true. type: boolean + firewall: + description: Firewall configuration. + properties: + defaultRulesManagement: + default: Managed + description: |- + DefaultRulesManagement determines the management policy for the default firewall rules + created by the controller. DefaultRulesManagement has no effect on user specified firewall + rules. DefaultRulesManagement has no effect when a HostProject is specified. + "Managed": The controller will create and manage firewall rules. + "Unmanaged": The controller will not create or modify any firewall rules. If + the RulesManagement is changed from Managed to Unmanaged after the firewall rules + have been created, then the firewall rules will not be deleted. + + Defaults to "Managed". + enum: + - Managed + - Unmanaged + type: string + type: object hostProject: description: HostProject is the name of the project hosting the shared VPC network resources. 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 573b66583..ce1b2edfd 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml @@ -175,6 +175,26 @@ spec: Defaults to true. type: boolean + firewall: + description: Firewall configuration. + properties: + defaultRulesManagement: + default: Managed + description: |- + DefaultRulesManagement determines the management policy for the default firewall rules + created by the controller. DefaultRulesManagement has no effect on user specified firewall + rules. DefaultRulesManagement has no effect when a HostProject is specified. + "Managed": The controller will create and manage firewall rules. + "Unmanaged": The controller will not create or modify any firewall rules. If + the RulesManagement is changed from Managed to Unmanaged after the firewall rules + have been created, then the firewall rules will not be deleted. + + Defaults to "Managed". + enum: + - Managed + - Unmanaged + type: string + type: object hostProject: description: HostProject is the name of the project hosting the shared VPC network resources. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclustertemplates.yaml index d527cb3f2..2c607f076 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclustertemplates.yaml @@ -169,6 +169,26 @@ spec: Defaults to true. type: boolean + firewall: + description: Firewall configuration. + properties: + defaultRulesManagement: + default: Managed + description: |- + DefaultRulesManagement determines the management policy for the default firewall rules + created by the controller. DefaultRulesManagement has no effect on user specified firewall + rules. DefaultRulesManagement has no effect when a HostProject is specified. + "Managed": The controller will create and manage firewall rules. + "Unmanaged": The controller will not create or modify any firewall rules. If + the RulesManagement is changed from Managed to Unmanaged after the firewall rules + have been created, then the firewall rules will not be deleted. + + Defaults to "Managed". + enum: + - Managed + - Unmanaged + type: string + type: object hostProject: description: HostProject is the name of the project hosting the shared VPC network resources.