diff --git a/api/v1beta1/azuremachine_validation_test.go b/api/v1beta1/azuremachine_validation_test.go index ae4fa9a9213..66f6764173f 100644 --- a/api/v1beta1/azuremachine_validation_test.go +++ b/api/v1beta1/azuremachine_validation_test.go @@ -59,6 +59,7 @@ func TestAzureMachine_ValidateSSHKey(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { err := ValidateSSHKey(tc.sshKey, field.NewPath("sshPublicKey")) + fmt.Println(err) if tc.wantErr { g.Expect(err).NotTo(BeEmpty()) } else { diff --git a/api/v1beta1/azuremanagedcontrolplane_default.go b/api/v1beta1/azuremanagedcontrolplane_default.go index fac7f9b9895..2ba3df70c8e 100644 --- a/api/v1beta1/azuremanagedcontrolplane_default.go +++ b/api/v1beta1/azuremanagedcontrolplane_default.go @@ -34,13 +34,13 @@ const ( // setDefaultSSHPublicKey sets the default SSHPublicKey for an AzureManagedControlPlane. func (m *AzureManagedControlPlane) setDefaultSSHPublicKey() error { - if sshKeyData := m.Spec.SSHPublicKey; sshKeyData == "" { + if sshKey := m.Spec.SSHPublicKey; sshKey != nil && *sshKey == "" { _, publicRsaKey, err := utilSSH.GenerateSSHKey() if err != nil { return err } - m.Spec.SSHPublicKey = base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey)) + m.Spec.SSHPublicKey = pointer.String(base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey))) } return nil diff --git a/api/v1beta1/azuremanagedcontrolplane_default_test.go b/api/v1beta1/azuremanagedcontrolplane_default_test.go index df91b4ffe59..e8a4bc9f0a3 100644 --- a/api/v1beta1/azuremanagedcontrolplane_default_test.go +++ b/api/v1beta1/azuremanagedcontrolplane_default_test.go @@ -36,11 +36,11 @@ func TestAzureManagedControlPlane_SetDefaultSSHPublicKey(t *testing.T) { err := publicKeyExistTest.m.setDefaultSSHPublicKey() g.Expect(err).To(BeNil()) - g.Expect(publicKeyExistTest.m.Spec.SSHPublicKey).To(Equal(existingPublicKey)) + g.Expect(*publicKeyExistTest.m.Spec.SSHPublicKey).To(Equal(existingPublicKey)) err = publicKeyNotExistTest.m.setDefaultSSHPublicKey() g.Expect(err).To(BeNil()) - g.Expect(publicKeyNotExistTest.m.Spec.SSHPublicKey).NotTo(BeEmpty()) + g.Expect(*publicKeyNotExistTest.m.Spec.SSHPublicKey).NotTo(BeEmpty()) } func createAzureManagedControlPlaneWithSSHPublicKey(sshPublicKey string) *AzureManagedControlPlane { @@ -50,7 +50,7 @@ func createAzureManagedControlPlaneWithSSHPublicKey(sshPublicKey string) *AzureM func hardcodedAzureManagedControlPlaneWithSSHKey(sshPublicKey string) *AzureManagedControlPlane { return &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - SSHPublicKey: sshPublicKey, + SSHPublicKey: &sshPublicKey, }, } } diff --git a/api/v1beta1/azuremanagedcontrolplane_types.go b/api/v1beta1/azuremanagedcontrolplane_types.go index 5da267e0b61..72703c53bae 100644 --- a/api/v1beta1/azuremanagedcontrolplane_types.go +++ b/api/v1beta1/azuremanagedcontrolplane_types.go @@ -124,8 +124,10 @@ type AzureManagedControlPlaneSpec struct { OutboundType *ManagedControlPlaneOutboundType `json:"outboundType,omitempty"` // SSHPublicKey is a string literal containing an ssh public key base64 encoded. + // Use empty string to autogenerate new key. Use null value to not set key. // Immutable. - SSHPublicKey string `json:"sshPublicKey"` + // +optional + SSHPublicKey *string `json:"sshPublicKey,omitempty"` // DNSServiceIP is an IP address assigned to the Kubernetes DNS service. // It must be within the Kubernetes service address range specified in serviceCidr. diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook.go b/api/v1beta1/azuremanagedcontrolplane_webhook.go index 1f5df9f8833..c6c399a3d54 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook.go @@ -307,9 +307,8 @@ func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error { // validateSSHKey validates an SSHKey. func (m *AzureManagedControlPlane) validateSSHKey(_ client.Client) error { - if m.Spec.SSHPublicKey != "" { - sshKey := m.Spec.SSHPublicKey - if errs := ValidateSSHKey(sshKey, field.NewPath("sshKey")); len(errs) > 0 { + if sshKey := m.Spec.SSHPublicKey; sshKey != nil && *sshKey != "" { + if errs := ValidateSSHKey(*sshKey, field.NewPath("sshKey")); len(errs) > 0 { return kerrors.NewAggregate(errs.ToAggregate().Errors()) } } diff --git a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go index 3137c0b9c15..3e11553d02c 100644 --- a/api/v1beta1/azuremanagedcontrolplane_webhook_test.go +++ b/api/v1beta1/azuremanagedcontrolplane_webhook_test.go @@ -41,6 +41,7 @@ func TestDefaultingWebhook(t *testing.T) { ResourceGroupName: "fooRg", Location: "fooLocation", Version: "1.17.5", + SSHPublicKey: pointer.String(""), }, } mcpw := &azureManagedControlPlaneWebhook{} @@ -49,7 +50,7 @@ func TestDefaultingWebhook(t *testing.T) { g.Expect(*amcp.Spec.NetworkPlugin).To(Equal("azure")) g.Expect(*amcp.Spec.LoadBalancerSKU).To(Equal("Standard")) g.Expect(amcp.Spec.Version).To(Equal("v1.17.5")) - g.Expect(amcp.Spec.SSHPublicKey).NotTo(BeEmpty()) + g.Expect(*amcp.Spec.SSHPublicKey).NotTo(BeEmpty()) g.Expect(amcp.Spec.NodeResourceGroupName).To(Equal("MC_fooRg_fooName_fooLocation")) g.Expect(amcp.Spec.VirtualNetwork.Name).To(Equal("fooName")) g.Expect(amcp.Spec.VirtualNetwork.Subnet.Name).To(Equal("fooName")) @@ -64,7 +65,7 @@ func TestDefaultingWebhook(t *testing.T) { amcp.Spec.LoadBalancerSKU = &lbSKU amcp.Spec.NetworkPolicy = &netPol amcp.Spec.Version = "9.99.99" - amcp.Spec.SSHPublicKey = "" + amcp.Spec.SSHPublicKey = nil amcp.Spec.NodeResourceGroupName = "fooNodeRg" amcp.Spec.VirtualNetwork.Name = "fooVnetName" amcp.Spec.VirtualNetwork.Subnet.Name = "fooSubnetName" @@ -76,7 +77,7 @@ func TestDefaultingWebhook(t *testing.T) { g.Expect(*amcp.Spec.LoadBalancerSKU).To(Equal(lbSKU)) g.Expect(*amcp.Spec.NetworkPolicy).To(Equal(netPol)) g.Expect(amcp.Spec.Version).To(Equal("v9.99.99")) - g.Expect(amcp.Spec.SSHPublicKey).NotTo(BeEmpty()) + g.Expect(amcp.Spec.SSHPublicKey).To(BeNil()) g.Expect(amcp.Spec.NodeResourceGroupName).To(Equal("fooNodeRg")) g.Expect(amcp.Spec.VirtualNetwork.Name).To(Equal("fooVnetName")) g.Expect(amcp.Spec.VirtualNetwork.Subnet.Name).To(Equal("fooSubnetName")) @@ -662,7 +663,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { Name: "microsoft-cluster", }, Spec: AzureManagedControlPlaneSpec{ - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), DNSServiceIP: pointer.String("192.168.0.0"), Version: "v1.23.5", }, @@ -677,7 +678,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { Name: "a-windows-cluster", }, Spec: AzureManagedControlPlaneSpec{ - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), DNSServiceIP: pointer.String("192.168.0.0"), Version: "v1.23.5", }, @@ -694,7 +695,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { }, DNSServiceIP: pointer.String("192.168.0.0"), Version: "v1.18.0", - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), AADProfile: &AADProfile{ Managed: true, AdminGroupObjectIDs: []string{ @@ -714,7 +715,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) { }, DNSServiceIP: pointer.String("192.168.0.0"), Version: "v1.18.0", - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), AADProfile: &AADProfile{ Managed: true, AdminGroupObjectIDs: []string{ @@ -774,7 +775,6 @@ func TestAzureManagedControlPlane_ValidateCreateFailure(t *testing.T) { func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { g := NewWithT(t) commonSSHKey := generateSSHPublicKey(true) - tests := []struct { name string oldAMCP *AzureManagedControlPlane @@ -882,14 +882,14 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { oldAMCP: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ DNSServiceIP: pointer.String("192.168.0.0"), - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), Version: "v1.18.0", }, }, amcp: &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ DNSServiceIP: pointer.String("192.168.0.0"), - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), Version: "v1.18.0", }, }, @@ -1319,7 +1319,7 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) { func createAzureManagedControlPlane(serviceIP, version, sshKey string) *AzureManagedControlPlane { return &AzureManagedControlPlane{ Spec: AzureManagedControlPlaneSpec{ - SSHPublicKey: sshKey, + SSHPublicKey: &sshKey, DNSServiceIP: pointer.String(serviceIP), Version: version, }, @@ -1331,7 +1331,7 @@ func getKnownValidAzureManagedControlPlane() *AzureManagedControlPlane { Spec: AzureManagedControlPlaneSpec{ DNSServiceIP: pointer.String("192.168.0.0"), Version: "v1.18.0", - SSHPublicKey: generateSSHPublicKey(true), + SSHPublicKey: pointer.String(generateSSHPublicKey(true)), AADProfile: &AADProfile{ Managed: true, AdminGroupObjectIDs: []string{ diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 4ed6bbeefaf..61103a07f89 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1175,6 +1175,11 @@ func (in *AzureManagedControlPlaneSpec) DeepCopyInto(out *AzureManagedControlPla *out = new(ManagedControlPlaneOutboundType) **out = **in } + if in.SSHPublicKey != nil { + in, out := &in.SSHPublicKey, &out.SSHPublicKey + *out = new(string) + **out = **in + } if in.DNSServiceIP != nil { in, out := &in.DNSServiceIP, &out.DNSServiceIP *out = new(string) diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 67bbc3e5d61..9bd328d048f 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -465,7 +465,6 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec() azure.ResourceSpecGetter Tags: s.ControlPlane.Spec.AdditionalTags, Headers: maps.FilterByKeyPrefix(s.ManagedClusterAnnotations(), infrav1.CustomHeaderPrefix), Version: strings.TrimPrefix(s.ControlPlane.Spec.Version, "v"), - SSHPublicKey: s.ControlPlane.Spec.SSHPublicKey, DNSServiceIP: s.ControlPlane.Spec.DNSServiceIP, VnetSubnetID: azure.SubnetID( s.ControlPlane.Spec.SubscriptionID, @@ -479,6 +478,9 @@ func (s *ManagedControlPlaneScope) ManagedClusterSpec() azure.ResourceSpecGetter KubeletUserAssignedIdentity: s.ControlPlane.Spec.KubeletUserAssignedIdentity, } + if s.ControlPlane.Spec.SSHPublicKey != nil { + managedClusterSpec.SSHPublicKey = *s.ControlPlane.Spec.SSHPublicKey + } if s.ControlPlane.Spec.NetworkPlugin != nil { managedClusterSpec.NetworkPlugin = *s.ControlPlane.Spec.NetworkPlugin } diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index 9af11d372b5..7985ff38fb5 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -266,14 +266,20 @@ func buildAutoScalerProfile(autoScalerProfile *AutoScalerProfile) *containerserv } // Parameters returns the parameters for the managed clusters. +// +//nolint:gocyclo // Function requires a lot of nil checks that raise complexity. func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{}) (params interface{}, err error) { ctx, log, done := tele.StartSpanWithLogger(ctx, "managedclusters.Service.Parameters") defer done() - decodedSSHPublicKey, err := base64.StdEncoding.DecodeString(s.SSHPublicKey) - if err != nil { - return nil, errors.Wrap(err, "failed to decode SSHPublicKey") + var decodedSSHPublicKey []byte + if s.SSHPublicKey != "" { + decodedSSHPublicKey, err = base64.StdEncoding.DecodeString(s.SSHPublicKey) + if err != nil { + return nil, errors.Wrap(err, "failed to decode SSHPublicKey") + } } + managedCluster := containerservice.ManagedCluster{ Identity: &containerservice.ManagedClusterIdentity{ Type: containerservice.ResourceIdentityTypeSystemAssigned, @@ -291,16 +297,7 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{ EnableRBAC: pointer.Bool(true), DNSPrefix: &s.Name, KubernetesVersion: &s.Version, - LinuxProfile: &containerservice.LinuxProfile{ - AdminUsername: pointer.String(azure.DefaultAKSUserName), - SSH: &containerservice.SSHConfiguration{ - PublicKeys: &[]containerservice.SSHPublicKey{ - { - KeyData: pointer.String(string(decodedSSHPublicKey)), - }, - }, - }, - }, + ServicePrincipalProfile: &containerservice.ManagedClusterServicePrincipalProfile{ ClientID: pointer.String("msi"), }, @@ -313,6 +310,19 @@ func (s *ManagedClusterSpec) Parameters(ctx context.Context, existing interface{ }, } + if decodedSSHPublicKey != nil { + managedCluster.LinuxProfile = &containerservice.LinuxProfile{ + AdminUsername: pointer.String(azure.DefaultAKSUserName), + SSH: &containerservice.SSHConfiguration{ + PublicKeys: &[]containerservice.SSHPublicKey{ + { + KeyData: pointer.String(string(decodedSSHPublicKey)), + }, + }, + }, + } + } + if s.PodCIDR != "" { managedCluster.NetworkProfile.PodCidr = &s.PodCIDR } diff --git a/azure/services/managedclusters/spec_test.go b/azure/services/managedclusters/spec_test.go index 940098da93f..b847a445182 100644 --- a/azure/services/managedclusters/spec_test.go +++ b/azure/services/managedclusters/spec_test.go @@ -18,6 +18,7 @@ package managedclusters import ( "context" + "encoding/base64" "testing" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2022-03-01/containerservice" @@ -69,6 +70,7 @@ func TestParameters(t *testing.T) { }, Version: "v1.22.0", LoadBalancerSKU: "Standard", + SSHPublicKey: base64.StdEncoding.EncodeToString([]byte("test-ssh-key")), GetAllAgentPools: func() ([]azure.ResourceSpecGetter, error) { return []azure.ResourceSpecGetter{ &agentpools.AgentPoolSpec{ @@ -163,6 +165,67 @@ func TestParameters(t *testing.T) { g.Expect(result).To(BeNil()) }, }, + { + name: "set Linux profile if SSH key is set", + existing: nil, + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: nil, + Version: "v1.22.0", + LoadBalancerSKU: "Standard", + SSHPublicKey: base64.StdEncoding.EncodeToString([]byte("test-ssh-key")), + GetAllAgentPools: func() ([]azure.ResourceSpecGetter, error) { + return []azure.ResourceSpecGetter{ + &agentpools.AgentPoolSpec{ + Name: "test-agentpool-0", + Mode: string(infrav1.NodePoolModeSystem), + ResourceGroup: "test-rg", + Replicas: int32(2), + AdditionalTags: map[string]string{ + "test-tag": "test-value", + }, + }, + }, nil + }, + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(containerservice.ManagedCluster{})) + g.Expect(result.(containerservice.ManagedCluster).LinuxProfile).To(Not(BeNil())) + g.Expect(result.(containerservice.ManagedCluster).LinuxProfile.SSH.PublicKeys).To(Not(BeNil())) + }, + }, + { + name: "skip Linux profile if SSH key is not set", + existing: nil, + spec: &ManagedClusterSpec{ + Name: "test-managedcluster", + ResourceGroup: "test-rg", + Location: "test-location", + Tags: nil, + Version: "v1.22.0", + LoadBalancerSKU: "Standard", + SSHPublicKey: "", + GetAllAgentPools: func() ([]azure.ResourceSpecGetter, error) { + return []azure.ResourceSpecGetter{ + &agentpools.AgentPoolSpec{ + Name: "test-agentpool-0", + Mode: string(infrav1.NodePoolModeSystem), + ResourceGroup: "test-rg", + Replicas: int32(2), + AdditionalTags: map[string]string{ + "test-tag": "test-value", + }, + }, + }, nil + }, + }, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(containerservice.ManagedCluster{})) + g.Expect(result.(containerservice.ManagedCluster).LinuxProfile).To(BeNil()) + }, + }, { name: "no update needed if both clusters have no authorized IP ranges", existing: getExistingClusterWithAPIServerAccessProfile(), @@ -307,7 +370,7 @@ func getSampleManagedCluster() containerservice.ManagedCluster { SSH: &containerservice.SSHConfiguration{ PublicKeys: &[]containerservice.SSHPublicKey{ { - KeyData: pointer.String(""), + KeyData: pointer.String("test-ssh-key"), }, }, }, @@ -316,7 +379,7 @@ func getSampleManagedCluster() containerservice.ManagedCluster { NodeResourceGroup: pointer.String("test-node-rg"), EnableRBAC: pointer.Bool(true), NetworkProfile: &containerservice.NetworkProfile{ - LoadBalancerSku: containerservice.LoadBalancerSku("Standard"), + LoadBalancerSku: "Standard", }, }, Identity: &containerservice.ManagedClusterIdentity{ diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index 2e6b1085cf9..a0a60c332ea 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -391,7 +391,8 @@ spec: type: object sshPublicKey: description: SSHPublicKey is a string literal containing an ssh public - key base64 encoded. Immutable. + key base64 encoded. Use empty string to autogenerate new key. Use + null value to not set key. Immutable. type: string subscriptionID: description: SubscriptionID is the GUID of the Azure subscription @@ -532,7 +533,6 @@ spec: required: - location - resourceGroupName - - sshPublicKey - version type: object status: