Skip to content

Commit

Permalink
merge to resolve conflicts with kubernetes-sigs#4008
Browse files Browse the repository at this point in the history
  • Loading branch information
nojnhuh committed Oct 14, 2023
2 parents 400fcfb + 7fc4734 commit 2fa8af2
Show file tree
Hide file tree
Showing 106 changed files with 2,276 additions and 180 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:

steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand All @@ -50,7 +50,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@ddccb873888234080b77e9bc2d4764d5ccaaccf9 # v2.21.9
uses: github/codeql-action/init@fdcae64e1484d349b3366718cdfef3d404390e85 # v2.22.1
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
Expand All @@ -60,7 +60,7 @@ jobs:
# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@ddccb873888234080b77e9bc2d4764d5ccaaccf9 # v2.21.9
uses: github/codeql-action/autobuild@fdcae64e1484d349b3366718cdfef3d404390e85 # v2.22.1

# ℹ️ Command-line programs to run using the OS shell.
# 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
Expand All @@ -73,6 +73,6 @@ jobs:
# ./location_of_script_within_repo/buildscript.sh

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@ddccb873888234080b77e9bc2d4764d5ccaaccf9 # v2.21.9
uses: github/codeql-action/analyze@fdcae64e1484d349b3366718cdfef3d404390e85 # v2.22.1
with:
category: "/language:${{matrix.language}}"
2 changes: 1 addition & 1 deletion .github/workflows/cover.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependabot-code-gen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/scorecards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:

steps:
- name: Harden Runner
uses: step-security/harden-runner@8ca2b8b2ece13480cda6dacd3511b49857a23c09 # v2.5.1
uses: step-security/harden-runner@1b05615854632b887b69ae1be8cbefe72d3ae423 # v2.6.0
with:
egress-policy: audit

Expand All @@ -41,7 +41,7 @@ jobs:
persist-credentials: false

- name: "Run analysis"
uses: ossf/scorecard-action@08b4669551908b1024bb425080c797723083c031 # v2.2.0
uses: ossf/scorecard-action@483ef80eb98fb506c348f7d62e28055e49fe2398 # v2.3.0
with:
results_file: results.sarif
results_format: sarif
Expand Down Expand Up @@ -71,6 +71,6 @@ jobs:

# Upload the results to GitHub's code scanning dashboard.
- name: "Upload to code-scanning"
uses: github/codeql-action/upload-sarif@ddccb873888234080b77e9bc2d4764d5ccaaccf9 # v2.21.9
uses: github/codeql-action/upload-sarif@fdcae64e1484d349b3366718cdfef3d404390e85 # v2.22.1
with:
sarif_file: results.sarif
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ GO_APIDIFF_VER := v0.6.0
GO_APIDIFF_BIN := go-apidiff
GO_APIDIFF := $(TOOLS_BIN_DIR)/$(GO_APIDIFF_BIN)

GINKGO_VER := v2.12.1
GINKGO_VER := v2.13.0
GINKGO_BIN := ginkgo
GINKGO := $(TOOLS_BIN_DIR)/$(GINKGO_BIN)-$(GINKGO_VER)

Expand Down Expand Up @@ -303,6 +303,9 @@ create-management-cluster: $(KUSTOMIZE) $(ENVSUBST) $(KUBECTL) $(KIND) ## Create
# Deploy CAPI
curl --retry $(CURL_RETRIES) -sSL https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.5.2/cluster-api-components.yaml | $(ENVSUBST) | $(KUBECTL) apply -f -

# Deploy CAAPH
curl --retry $(CURL_RETRIES) -sSL https://github.com/kubernetes-sigs/cluster-api-addon-provider-helm/releases/download/v0.1.0-alpha.9/addon-components.yaml | $(ENVSUBST) | $(KUBECTL) apply -f -

# Deploy CAPZ
$(KIND) load docker-image $(CONTROLLER_IMG)-$(ARCH):$(TAG) --name=$(KIND_CLUSTER_NAME)
$(KUSTOMIZE) build config/default | $(ENVSUBST) | $(KUBECTL) apply -f - --server-side=true
Expand All @@ -312,6 +315,9 @@ create-management-cluster: $(KUSTOMIZE) $(ENVSUBST) $(KUBECTL) $(KIND) ## Create
$(KUBECTL) wait --for=condition=Available --timeout=5m -n capi-kubeadm-bootstrap-system deployment -l cluster.x-k8s.io/provider=bootstrap-kubeadm
$(KUBECTL) wait --for=condition=Available --timeout=5m -n capi-kubeadm-control-plane-system deployment -l cluster.x-k8s.io/provider=control-plane-kubeadm

# Wait for CAAPH deployment
$(KUBECTL) wait --for=condition=Available --timeout=5m -n caaph-system deployment -l cluster.x-k8s.io/provider=helm

# Wait for the ClusterResourceSet CRD resource to be "installed" onto the mgmt cluster before installing CRS addons
timeout --foreground 300 bash -c "until $(KUBECTL) get clusterresourcesets -A; do sleep 3; done"

Expand Down
46 changes: 46 additions & 0 deletions api/v1beta1/azuremachine_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
. "github.com/onsi/gomega"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand Down Expand Up @@ -90,6 +91,13 @@ func TestAzureMachineSpec_SetIdentityDefaults(t *testing.T) {
Identity: VMIdentitySystemAssigned,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{},
}}}
bothDeprecatedRoleAssignmentNameAndSystemAssignedIdentityRoleTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
Identity: VMIdentitySystemAssigned,
RoleAssignmentName: existingRoleAssignmentName,
SystemAssignedIdentityRole: &SystemAssignedIdentityRole{
Name: existingRoleAssignmentName,
},
}}}

roleAssignmentExistTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(roleAssignmentExistTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Equal(existingRoleAssignmentName))
Expand All @@ -111,6 +119,44 @@ func TestAzureMachineSpec_SetIdentityDefaults(t *testing.T) {
g.Expect(err).To(Not(HaveOccurred()))
g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.Scope).To(Equal(fmt.Sprintf("/subscriptions/%s/", fakeSubscriptionID)))
g.Expect(emptyTest.machine.Spec.SystemAssignedIdentityRole.DefinitionID).To(Equal(fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Authorization/roleDefinitions/%s", fakeSubscriptionID, ContributorRoleID)))

bothDeprecatedRoleAssignmentNameAndSystemAssignedIdentityRoleTest.machine.Spec.SetIdentityDefaults(fakeSubscriptionID)
g.Expect(bothDeprecatedRoleAssignmentNameAndSystemAssignedIdentityRoleTest.machine.Spec.RoleAssignmentName).To(Not(BeEmpty()))
g.Expect(bothDeprecatedRoleAssignmentNameAndSystemAssignedIdentityRoleTest.machine.Spec.SystemAssignedIdentityRole.Name).To(Not(BeEmpty()))
}

func TestAzureMachineSpec_SetSpotEvictionPolicyDefaults(t *testing.T) {
deallocatePolicy := SpotEvictionPolicyDeallocate
deletePolicy := SpotEvictionPolicyDelete

g := NewWithT(t)

type test struct {
machine *AzureMachine
}

spotVMOptionsExistTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
SpotVMOptions: &SpotVMOptions{
MaxPrice: &resource.Quantity{Format: "vmoptions-0"},
},
}}}

localDiffDiskSettingsExistTest := test{machine: &AzureMachine{Spec: AzureMachineSpec{
SpotVMOptions: &SpotVMOptions{
MaxPrice: &resource.Quantity{},
},
OSDisk: OSDisk{
DiffDiskSettings: &DiffDiskSettings{
Option: "Local",
},
},
}}}

spotVMOptionsExistTest.machine.Spec.SetSpotEvictionPolicyDefaults()
g.Expect(spotVMOptionsExistTest.machine.Spec.SpotVMOptions.EvictionPolicy).To(Equal(&deallocatePolicy))

localDiffDiskSettingsExistTest.machine.Spec.SetSpotEvictionPolicyDefaults()
g.Expect(localDiffDiskSettingsExistTest.machine.Spec.SpotVMOptions.EvictionPolicy).To(Equal(&deletePolicy))
}

func TestAzureMachineSpec_SetDataDisksDefaults(t *testing.T) {
Expand Down
15 changes: 15 additions & 0 deletions api/v1beta1/azuremachine_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,22 @@ func TestAzureMachine_ValidateUpdate(t *testing.T) {
},
wantErr: true,
},
{
name: "invalidtest: updating subnet name from empty to non empty",
oldMachine: &AzureMachine{
Spec: AzureMachineSpec{
NetworkInterfaces: []NetworkInterface{{SubnetName: ""}},
},
},
newMachine: &AzureMachine{
Spec: AzureMachineSpec{
NetworkInterfaces: []NetworkInterface{{SubnetName: "subnet1"}},
},
},
wantErr: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
mw := &azureMachineWebhook{}
Expand Down
3 changes: 3 additions & 0 deletions api/v1beta1/azuremanagedcontrolplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ type AzureManagedControlPlaneSpec struct {
// Immutable.
// +optional
DNSPrefix *string `json:"dnsPrefix,omitempty"`
// DisableLocalAccounts disables getting static credentials for this cluster when set. Expected to only be used for AAD clusters.
// +optional
DisableLocalAccounts *bool `json:"disableLocalAccounts,omitempty"`
}

// HTTPProxyConfig is the HTTP proxy configuration for the cluster.
Expand Down
96 changes: 69 additions & 27 deletions api/v1beta1/azuremanagedcontrolplane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,31 +234,6 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
allErrs = append(allErrs, err)
}

if old.Spec.AADProfile != nil {
if m.Spec.AADProfile == nil {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile"),
m.Spec.AADProfile,
"field cannot be nil, cannot disable AADProfile"))
} else {
if !m.Spec.AADProfile.Managed && old.Spec.AADProfile.Managed {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile.Managed"),
m.Spec.AADProfile.Managed,
"cannot set AADProfile.Managed to false"))
}
if len(m.Spec.AADProfile.AdminGroupObjectIDs) == 0 {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile.AdminGroupObjectIDs"),
m.Spec.AADProfile.AdminGroupObjectIDs,
"length of AADProfile.AdminGroupObjectIDs cannot be zero"))
}
}
}

if err := webhookutils.ValidateImmutable(
field.NewPath("Spec", "DNSPrefix"),
m.Spec.DNSPrefix,
Expand Down Expand Up @@ -289,6 +264,10 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
allErrs = append(allErrs, errs...)
}

if errs := m.validateAADProfileUpdateAndLocalAccounts(old); len(errs) > 0 {
allErrs = append(allErrs, errs...)
}

if errs := m.validateOIDCIssuerProfileUpdate(old); len(errs) > 0 {
allErrs = append(allErrs, errs...)
}
Expand Down Expand Up @@ -318,6 +297,7 @@ func (m *AzureManagedControlPlane) Validate(cli client.Client) error {
m.validateIdentity,
m.validateNetworkPluginMode,
m.validateDNSPrefix,
m.validateDisableLocalAccounts,
}

var errs []error
Expand Down Expand Up @@ -350,6 +330,14 @@ func (m *AzureManagedControlPlane) validateDNSPrefix(_ client.Client) error {
return kerrors.NewAggregate(allErrs.ToAggregate().Errors())
}

// validateVersion disabling local accounts for AAD based clusters.
func (m *AzureManagedControlPlane) validateDisableLocalAccounts(_ client.Client) error {
if m.Spec.DisableLocalAccounts != nil && m.Spec.AADProfile == nil {
return errors.New("DisableLocalAccounts should be set only for AAD enabled clusters")
}
return nil
}

// validateVersion validates the Kubernetes version.
func (m *AzureManagedControlPlane) validateVersion(_ client.Client) error {
if !kubeSemver.MatchString(m.Spec.Version) {
Expand Down Expand Up @@ -600,8 +588,62 @@ func (m *AzureManagedControlPlane) validateVirtualNetworkUpdate(old *AzureManage
func (m *AzureManagedControlPlane) validateNetworkPluginModeUpdate(old *AzureManagedControlPlane) field.ErrorList {
var allErrs field.ErrorList

if ptr.Deref(m.Spec.NetworkPluginMode, "") == NetworkPluginModeOverlay && old.Spec.NetworkPolicy != nil {
allErrs = append(allErrs, field.Forbidden(field.NewPath("Spec", "NetworkPluginMode"), fmt.Sprintf("%q NetworkPolicyMode cannot be enabled when NetworkPolicy is set", NetworkPluginModeOverlay)))
if ptr.Deref(old.Spec.NetworkPluginMode, "") != NetworkPluginModeOverlay &&
ptr.Deref(m.Spec.NetworkPluginMode, "") == NetworkPluginModeOverlay &&
old.Spec.NetworkPolicy != nil {
allErrs = append(allErrs, field.Forbidden(field.NewPath("Spec", "NetworkPluginMode"), fmt.Sprintf("%q NetworkPluginMode cannot be enabled when NetworkPolicy is set", NetworkPluginModeOverlay)))
}

return allErrs
}

// validateAADProfileUpdateAndLocalAccounts validates updates for AADProfile.
func (m *AzureManagedControlPlane) validateAADProfileUpdateAndLocalAccounts(old *AzureManagedControlPlane) field.ErrorList {
var allErrs field.ErrorList
if old.Spec.AADProfile != nil {
if m.Spec.AADProfile == nil {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile"),
m.Spec.AADProfile,
"field cannot be nil, cannot disable AADProfile"))
} else {
if !m.Spec.AADProfile.Managed && old.Spec.AADProfile.Managed {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile.Managed"),
m.Spec.AADProfile.Managed,
"cannot set AADProfile.Managed to false"))
}
if len(m.Spec.AADProfile.AdminGroupObjectIDs) == 0 {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "AADProfile.AdminGroupObjectIDs"),
m.Spec.AADProfile.AdminGroupObjectIDs,
"length of AADProfile.AdminGroupObjectIDs cannot be zero"))
}
}
}

if old.Spec.DisableLocalAccounts == nil &&
m.Spec.DisableLocalAccounts != nil &&
m.Spec.AADProfile == nil {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "DisableLocalAccounts"),
m.Spec.DisableLocalAccounts,
"DisableLocalAccounts can be set only for AAD enabled clusters"))
}

if old.Spec.DisableLocalAccounts != nil {
// Prevent DisableLocalAccounts modification if it was already set to some value
if err := webhookutils.ValidateImmutable(
field.NewPath("Spec", "DisableLocalAccounts"),
m.Spec.DisableLocalAccounts,
old.Spec.DisableLocalAccounts,
); err != nil {
allErrs = append(allErrs, err)
}
}

return allErrs
Expand Down
Loading

0 comments on commit 2fa8af2

Please sign in to comment.