From 682457afedadbdf67dfeca5da73ae7b33615b878 Mon Sep 17 00:00:00 2001 From: Cong Yue Date: Mon, 14 Aug 2023 17:13:57 -0700 Subject: [PATCH] Support master authorized network config (#65) * add MasterAuthorizedNetworksConfig filed into GCPManagedControlPlane CRD * fix * disable it if no desired specified in CR * change to do not reconcile if the desired CR is nil * add comment add nil handling fo GcpPublicCidrsAccessEnabled change nil as enabled fix display_name not to be optional address comments fix typo and remove empty line --- .../services/container/clusters/reconcile.go | 87 ++++++++++++++++++- ...ster.x-k8s.io_gcpmanagedcontrolplanes.yaml | 27 ++++++ .../v1beta1/gcpmanagedcontrolplane_types.go | 27 ++++++ exp/api/v1beta1/zz_generated.deepcopy.go | 51 +++++++++++ 4 files changed, 188 insertions(+), 4 deletions(-) diff --git a/cloud/services/container/clusters/reconcile.go b/cloud/services/container/clusters/reconcile.go index 4317e058b..89894f50f 100644 --- a/cloud/services/container/clusters/reconcile.go +++ b/cloud/services/container/clusters/reconcile.go @@ -25,6 +25,7 @@ import ( "cloud.google.com/go/container/apiv1/containerpb" "github.com/go-logr/logr" + "github.com/google/go-cmp/cmp" "github.com/googleapis/gax-go/v2/apierror" "github.com/pkg/errors" "google.golang.org/grpc/codes" @@ -136,7 +137,7 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) { return ctrl.Result{}, statusErr } - needUpdate, updateClusterRequest := s.checkDiffAndPrepareUpdate(cluster) + needUpdate, updateClusterRequest := s.checkDiffAndPrepareUpdate(cluster, &log) if needUpdate { log.Info("Update required") err = s.updateCluster(ctx, updateClusterRequest, &log) @@ -258,6 +259,7 @@ func (s *Service) createCluster(ctx context.Context, log *logr.Logger) error { ReleaseChannel: &containerpb.ReleaseChannel{ Channel: convertToSdkReleaseChannel(s.scope.GCPManagedControlPlane.Spec.ReleaseChannel), }, + MasterAuthorizedNetworksConfig: convertToSdkMasterAuthorizedNetworksConfig(s.scope.GCPManagedControlPlane.Spec.MasterAuthorizedNetworksConfig), } if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil { cluster.InitialClusterVersion = *s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion @@ -320,25 +322,102 @@ func convertToSdkReleaseChannel(channel *infrav1exp.ReleaseChannel) containerpb. } } -func (s *Service) checkDiffAndPrepareUpdate(existingCluster *containerpb.Cluster) (bool, *containerpb.UpdateClusterRequest) { +// convertToSdkMasterAuthorizedNetworksConfig converts the MasterAuthorizedNetworksConfig defined in CRs to the SDK version. +func convertToSdkMasterAuthorizedNetworksConfig(config *infrav1exp.MasterAuthorizedNetworksConfig) *containerpb.MasterAuthorizedNetworksConfig { + // if config is nil, it means that the user wants to disable the feature. + if config == nil { + return &containerpb.MasterAuthorizedNetworksConfig{ + Enabled: false, + CidrBlocks: []*containerpb.MasterAuthorizedNetworksConfig_CidrBlock{}, + GcpPublicCidrsAccessEnabled: new(bool), + } + } + + // Convert the CidrBlocks slice. + cidrBlocks := make([]*containerpb.MasterAuthorizedNetworksConfig_CidrBlock, len(config.CidrBlocks)) + for i, cidrBlock := range config.CidrBlocks { + cidrBlocks[i] = &containerpb.MasterAuthorizedNetworksConfig_CidrBlock{ + CidrBlock: cidrBlock.CidrBlock, + DisplayName: cidrBlock.DisplayName, + } + } + + return &containerpb.MasterAuthorizedNetworksConfig{ + Enabled: true, + CidrBlocks: cidrBlocks, + GcpPublicCidrsAccessEnabled: config.GcpPublicCidrsAccessEnabled, + } +} + +func (s *Service) checkDiffAndPrepareUpdate(existingCluster *containerpb.Cluster, log *logr.Logger) (bool, *containerpb.UpdateClusterRequest) { + log.V(4).Info("Checking diff and preparing update.") + needUpdate := false clusterUpdate := containerpb.ClusterUpdate{} // Release channel desiredReleaseChannel := convertToSdkReleaseChannel(s.scope.GCPManagedControlPlane.Spec.ReleaseChannel) if desiredReleaseChannel != existingCluster.ReleaseChannel.Channel { + log.V(2).Info("Release channel update required", "current", existingCluster.ReleaseChannel.Channel, "desired", desiredReleaseChannel) needUpdate = true clusterUpdate.DesiredReleaseChannel = &containerpb.ReleaseChannel{ Channel: desiredReleaseChannel, } } // Master version - if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil && *s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != existingCluster.InitialClusterVersion { + if s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion != nil { + desiredMasterVersion := *s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion + if desiredMasterVersion != existingCluster.InitialClusterVersion { + needUpdate = true + clusterUpdate.DesiredMasterVersion = desiredMasterVersion + log.V(2).Info("Master version update required", "current", existingCluster.InitialClusterVersion, "desired", desiredMasterVersion) + } + } + + // DesiredMasterAuthorizedNetworksConfig + // When desiredMasterAuthorizedNetworksConfig is nil, it means that the user wants to disable the feature. + desiredMasterAuthorizedNetworksConfig := convertToSdkMasterAuthorizedNetworksConfig(s.scope.GCPManagedControlPlane.Spec.MasterAuthorizedNetworksConfig) + if !compareMasterAuthorizedNetworksConfig(desiredMasterAuthorizedNetworksConfig, existingCluster.MasterAuthorizedNetworksConfig) { needUpdate = true - clusterUpdate.DesiredMasterVersion = *s.scope.GCPManagedControlPlane.Spec.ControlPlaneVersion + clusterUpdate.DesiredMasterAuthorizedNetworksConfig = desiredMasterAuthorizedNetworksConfig + log.V(2).Info("Master authorized networks config update required", "current", existingCluster.MasterAuthorizedNetworksConfig, "desired", desiredMasterAuthorizedNetworksConfig) + } + log.V(4).Info("Master authorized networks config update check", "current", existingCluster.MasterAuthorizedNetworksConfig) + if desiredMasterAuthorizedNetworksConfig != nil { + log.V(4).Info("Master authorized networks config update check", "desired", desiredMasterAuthorizedNetworksConfig) } + updateClusterRequest := containerpb.UpdateClusterRequest{ Name: s.scope.ClusterFullName(), Update: &clusterUpdate, } + log.V(4).Info("Update cluster request. ", "needUpdate", needUpdate, "updateClusterRequest", &updateClusterRequest) return needUpdate, &updateClusterRequest } + +// compare if two MasterAuthorizedNetworksConfig are equal. +func compareMasterAuthorizedNetworksConfig(a, b *containerpb.MasterAuthorizedNetworksConfig) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + + if a.Enabled != b.Enabled { + return false + } + if (a.GcpPublicCidrsAccessEnabled == nil && b.GcpPublicCidrsAccessEnabled != nil) || (a.GcpPublicCidrsAccessEnabled != nil && b.GcpPublicCidrsAccessEnabled == nil) { + return false + } + if a.GcpPublicCidrsAccessEnabled != nil && b.GcpPublicCidrsAccessEnabled != nil && *a.GcpPublicCidrsAccessEnabled != *b.GcpPublicCidrsAccessEnabled { + return false + } + // if one cidrBlocks is nil, but the other is empty, they are equal. + if (a.CidrBlocks == nil && b.CidrBlocks != nil && len(b.CidrBlocks) == 0) || (b.CidrBlocks == nil && a.CidrBlocks != nil && len(a.CidrBlocks) == 0) { + return true + } + if !cmp.Equal(a.CidrBlocks, b.CidrBlocks) { + return false + } + return true +} diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml index 57cb0b131..22f7d4440 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedcontrolplanes.yaml @@ -91,6 +91,33 @@ spec: description: Location represents the location (region or zone) in which the GKE cluster will be created. type: string + master_authorized_networks_config: + description: MasterAuthorizedNetworksConfig represents configuration + options for master authorized networks feature of the GKE cluster. + This feature is disabled if this field is not specified. + properties: + cidr_blocks: + description: cidr_blocks define up to 50 external networks that + could access Kubernetes master through HTTPS. + items: + description: MasterAuthorizedNetworksConfigCidrBlock contains + an optional name and one CIDR block. + properties: + cidr_block: + description: cidr_block must be specified in CIDR notation. + pattern: ^(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/([0-9]|[1-2][0-9]|3[0-2]))?$|^([a-fA-F0-9:]+:+)+[a-fA-F0-9]+\/[0-9]{1,3}$ + type: string + display_name: + description: display_name is an field for users to identify + CIDR blocks. + type: string + type: object + type: array + gcp_public_cidrs_access_enabled: + description: Whether master is accessible via Google Compute Engine + Public IP addresses. + type: boolean + type: object project: description: Project is the name of the project to deploy the cluster to. diff --git a/exp/api/v1beta1/gcpmanagedcontrolplane_types.go b/exp/api/v1beta1/gcpmanagedcontrolplane_types.go index 935c22aac..b0fb6540d 100644 --- a/exp/api/v1beta1/gcpmanagedcontrolplane_types.go +++ b/exp/api/v1beta1/gcpmanagedcontrolplane_types.go @@ -53,6 +53,10 @@ type GCPManagedControlPlaneSpec struct { // Endpoint represents the endpoint used to communicate with the control plane. // +optional Endpoint clusterv1.APIEndpoint `json:"endpoint"` + // MasterAuthorizedNetworksConfig represents configuration options for master authorized networks feature of the GKE cluster. + // This feature is disabled if this field is not specified. + // +optional + MasterAuthorizedNetworksConfig *MasterAuthorizedNetworksConfig `json:"master_authorized_networks_config,omitempty"` } // GCPManagedControlPlaneStatus defines the observed state of GCPManagedControlPlane. @@ -115,6 +119,29 @@ const ( Stable ReleaseChannel = "stable" ) +// MasterAuthorizedNetworksConfig contains configuration options for the master authorized networks feature. +// Enabled master authorized networks will disallow all external traffic to access +// Kubernetes master through HTTPS except traffic from the given CIDR blocks, +// Google Compute Engine Public IPs and Google Prod IPs. +type MasterAuthorizedNetworksConfig struct { + // cidr_blocks define up to 50 external networks that could access + // Kubernetes master through HTTPS. + // +optional + CidrBlocks []*MasterAuthorizedNetworksConfigCidrBlock `json:"cidr_blocks,omitempty"` + // Whether master is accessible via Google Compute Engine Public IP addresses. + // +optional + GcpPublicCidrsAccessEnabled *bool `json:"gcp_public_cidrs_access_enabled,omitempty"` +} + +// MasterAuthorizedNetworksConfigCidrBlock contains an optional name and one CIDR block. +type MasterAuthorizedNetworksConfigCidrBlock struct { + // display_name is an field for users to identify CIDR blocks. + DisplayName string `json:"display_name,omitempty"` + // cidr_block must be specified in CIDR notation. + // +kubebuilder:validation:Pattern=`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}(?:\/([0-9]|[1-2][0-9]|3[0-2]))?$|^([a-fA-F0-9:]+:+)+[a-fA-F0-9]+\/[0-9]{1,3}$` + CidrBlock string `json:"cidr_block,omitempty"` +} + // GetConditions returns the control planes conditions. func (r *GCPManagedControlPlane) GetConditions() clusterv1.Conditions { return r.Status.Conditions diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go index 1e0e020be..1e2b33ec7 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -218,6 +218,11 @@ func (in *GCPManagedControlPlaneSpec) DeepCopyInto(out *GCPManagedControlPlaneSp **out = **in } out.Endpoint = in.Endpoint + if in.MasterAuthorizedNetworksConfig != nil { + in, out := &in.MasterAuthorizedNetworksConfig, &out.MasterAuthorizedNetworksConfig + *out = new(MasterAuthorizedNetworksConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GCPManagedControlPlaneSpec. @@ -377,6 +382,52 @@ func (in *GCPManagedMachinePoolStatus) DeepCopy() *GCPManagedMachinePoolStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MasterAuthorizedNetworksConfig) DeepCopyInto(out *MasterAuthorizedNetworksConfig) { + *out = *in + if in.CidrBlocks != nil { + in, out := &in.CidrBlocks, &out.CidrBlocks + *out = make([]*MasterAuthorizedNetworksConfigCidrBlock, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(MasterAuthorizedNetworksConfigCidrBlock) + **out = **in + } + } + } + if in.GcpPublicCidrsAccessEnabled != nil { + in, out := &in.GcpPublicCidrsAccessEnabled, &out.GcpPublicCidrsAccessEnabled + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MasterAuthorizedNetworksConfig. +func (in *MasterAuthorizedNetworksConfig) DeepCopy() *MasterAuthorizedNetworksConfig { + if in == nil { + return nil + } + out := new(MasterAuthorizedNetworksConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MasterAuthorizedNetworksConfigCidrBlock) DeepCopyInto(out *MasterAuthorizedNetworksConfigCidrBlock) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MasterAuthorizedNetworksConfigCidrBlock. +func (in *MasterAuthorizedNetworksConfigCidrBlock) DeepCopy() *MasterAuthorizedNetworksConfigCidrBlock { + if in == nil { + return nil + } + out := new(MasterAuthorizedNetworksConfigCidrBlock) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodePoolAutoScaling) DeepCopyInto(out *NodePoolAutoScaling) { *out = *in