Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support master authorized network config #1004

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 83 additions & 4 deletions cloud/services/container/clusters/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Comment on lines +331 to +332
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could we just omit these and leave Enabled: false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not work per my test. for GcpPublicCidrsAccessEnabled, it will be Nil instead of false as it is a *bool rather than bool in the GKE SDK. similarly to CidrBlocks, it will be Nil if we do not put some empty values. And it is a Nil to the GKE SDK, it will not overwrite current values and cause us to always have diff on those two fields when reconciling. Let me know if you have different opinions on this.

}
}

// 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 27 additions & 0 deletions exp/api/v1beta1/gcpmanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"`
richardcase marked this conversation as resolved.
Show resolved Hide resolved
// Whether master is accessible via Google Compute Engine Public IP addresses.
// +optional
GcpPublicCidrsAccessEnabled *bool `json:"gcp_public_cidrs_access_enabled,omitempty"`
richardcase marked this conversation as resolved.
Show resolved Hide resolved
}

// 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"`
richardcase marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down
51 changes: 51 additions & 0 deletions exp/api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading