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

Allow skipping Linux profile on managed clusters #3677

Merged
merged 1 commit into from
Jul 18, 2023
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
4 changes: 2 additions & 2 deletions api/v1beta1/azuremanagedcontrolplane_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
6 changes: 3 additions & 3 deletions api/v1beta1/azuremanagedcontrolplane_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -50,7 +50,7 @@ func createAzureManagedControlPlaneWithSSHPublicKey(sshPublicKey string) *AzureM
func hardcodedAzureManagedControlPlaneWithSSHKey(sshPublicKey string) *AzureManagedControlPlane {
return &AzureManagedControlPlane{
Spec: AzureManagedControlPlaneSpec{
SSHPublicKey: sshPublicKey,
SSHPublicKey: &sshPublicKey,
},
}
}
Expand Down
4 changes: 3 additions & 1 deletion api/v1beta1/azuremanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
5 changes: 2 additions & 3 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
24 changes: 12 additions & 12 deletions api/v1beta1/azuremanagedcontrolplane_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestDefaultingWebhook(t *testing.T) {
ResourceGroupName: "fooRg",
Location: "fooLocation",
Version: "1.17.5",
SSHPublicKey: pointer.String(""),
},
}
mcpw := &azureManagedControlPlaneWebhook{}
Expand All @@ -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"))
Expand All @@ -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"
Expand All @@ -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"))
Expand Down Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
},
},
Expand Down Expand Up @@ -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,
},
Expand All @@ -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{
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

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

4 changes: 3 additions & 1 deletion azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,6 @@
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,
Expand All @@ -479,6 +478,9 @@
KubeletUserAssignedIdentity: s.ControlPlane.Spec.KubeletUserAssignedIdentity,
}

if s.ControlPlane.Spec.SSHPublicKey != nil {
managedClusterSpec.SSHPublicKey = *s.ControlPlane.Spec.SSHPublicKey
}

Check warning on line 483 in azure/scope/managedcontrolplane.go

View check run for this annotation

Codecov / codecov/patch

azure/scope/managedcontrolplane.go#L482-L483

Added lines #L482 - L483 were not covered by tests
if s.ControlPlane.Spec.NetworkPlugin != nil {
managedClusterSpec.NetworkPlugin = *s.ControlPlane.Spec.NetworkPlugin
}
Expand Down
36 changes: 23 additions & 13 deletions azure/services/managedclusters/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,20 @@
}

// 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")
}

Check warning on line 280 in azure/services/managedclusters/spec.go

View check run for this annotation

Codecov / codecov/patch

azure/services/managedclusters/spec.go#L279-L280

Added lines #L279 - L280 were not covered by tests
}

managedCluster := containerservice.ManagedCluster{
Identity: &containerservice.ManagedClusterIdentity{
Type: containerservice.ResourceIdentityTypeSystemAssigned,
Expand All @@ -291,16 +297,7 @@
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"),
},
Expand All @@ -313,6 +310,19 @@
},
}

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
}
Expand Down
67 changes: 65 additions & 2 deletions azure/services/managedclusters/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)[0].KeyData).To(Equal("test-ssh-key"))
},
},
{
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(),
Expand Down Expand Up @@ -307,7 +370,7 @@ func getSampleManagedCluster() containerservice.ManagedCluster {
SSH: &containerservice.SSHConfiguration{
PublicKeys: &[]containerservice.SSHPublicKey{
{
KeyData: pointer.String(""),
KeyData: pointer.String("test-ssh-key"),
},
},
},
Expand All @@ -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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -532,7 +533,6 @@ spec:
required:
- location
- resourceGroupName
- sshPublicKey
- version
type: object
status:
Expand Down