Skip to content

Commit e501d4b

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3359 from jackfrancis/aks-rm-create-webhooks-controplaneendpoint
remove strict AKS create validations for spec.controlPlaneEndpoint
2 parents 1676fca + 87602d5 commit e501d4b

File tree

7 files changed

+24
-191
lines changed

7 files changed

+24
-191
lines changed

‎api/v1beta1/azuremanagedcluster_webhook.go‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"k8s.io/apimachinery/pkg/util/validation/field"
2626
"sigs.k8s.io/cluster-api-provider-azure/feature"
2727
"sigs.k8s.io/cluster-api-provider-azure/util/maps"
28-
webhookutils "sigs.k8s.io/cluster-api-provider-azure/util/webhook"
2928
capifeature "sigs.k8s.io/cluster-api/feature"
3029
ctrl "sigs.k8s.io/controller-runtime"
3130
"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -52,18 +51,6 @@ func (r *AzureManagedCluster) ValidateCreate() error {
5251
"can be set only if the Cluster API 'MachinePool' feature flag is enabled",
5352
)
5453
}
55-
if r.Spec.ControlPlaneEndpoint.Host != "" {
56-
return field.Forbidden(
57-
field.NewPath("Spec", "ControlPlaneEndpoint", "Host"),
58-
controlPlaneEndpointErrorMessage,
59-
)
60-
}
61-
if r.Spec.ControlPlaneEndpoint.Port != 0 {
62-
return field.Forbidden(
63-
field.NewPath("Spec", "ControlPlaneEndpoint", "Port"),
64-
controlPlaneEndpointErrorMessage,
65-
)
66-
}
6754
return nil
6855
}
6956

@@ -83,24 +70,6 @@ func (r *AzureManagedCluster) ValidateUpdate(oldRaw runtime.Object) error {
8370
fmt.Sprintf("annotations with '%s' prefix are immutable", CustomHeaderPrefix)))
8471
}
8572

86-
if old.Spec.ControlPlaneEndpoint.Host != "" {
87-
if err := webhookutils.ValidateImmutable(
88-
field.NewPath("Spec", "ControlPlaneEndpoint", "Host"),
89-
old.Spec.ControlPlaneEndpoint.Host,
90-
r.Spec.ControlPlaneEndpoint.Host); err != nil {
91-
allErrs = append(allErrs, err)
92-
}
93-
}
94-
95-
if old.Spec.ControlPlaneEndpoint.Port != 0 {
96-
if err := webhookutils.ValidateImmutable(
97-
field.NewPath("Spec", "ControlPlaneEndpoint", "Port"),
98-
old.Spec.ControlPlaneEndpoint.Port,
99-
r.Spec.ControlPlaneEndpoint.Port); err != nil {
100-
allErrs = append(allErrs, err)
101-
}
102-
}
103-
10473
if len(allErrs) != 0 {
10574
return apierrors.NewInvalid(GroupVersion.WithKind("AzureManagedCluster").GroupKind(), r.Name, allErrs)
10675
}

‎api/v1beta1/azuremanagedcluster_webhook_test.go‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,12 @@ func TestAzureManagedCluster_ValidateUpdate(t *testing.T) {
122122
wantErr: false,
123123
},
124124
{
125-
name: "ControlPlaneEndpoint.Port is immutable",
125+
name: "ControlPlaneEndpoint.Port update (AKS API-derived update scenario)",
126126
oldAMC: &AzureManagedCluster{
127127
ObjectMeta: metav1.ObjectMeta{},
128128
Spec: AzureManagedClusterSpec{
129129
ControlPlaneEndpoint: clusterv1.APIEndpoint{
130130
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
131-
Port: 443,
132131
},
133132
},
134133
},
@@ -137,42 +136,22 @@ func TestAzureManagedCluster_ValidateUpdate(t *testing.T) {
137136
Spec: AzureManagedClusterSpec{
138137
ControlPlaneEndpoint: clusterv1.APIEndpoint{
139138
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
140-
Port: 444,
139+
Port: 443,
141140
},
142141
},
143142
},
144-
wantErr: true,
143+
wantErr: false,
145144
},
146145
{
147-
name: "ControlPlaneEndpoint.Host is immutable",
146+
name: "ControlPlaneEndpoint.Host update (AKS API-derived update scenario)",
148147
oldAMC: &AzureManagedCluster{
149148
ObjectMeta: metav1.ObjectMeta{},
150149
Spec: AzureManagedClusterSpec{
151150
ControlPlaneEndpoint: clusterv1.APIEndpoint{
152-
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
153151
Port: 443,
154152
},
155153
},
156154
},
157-
amc: &AzureManagedCluster{
158-
ObjectMeta: metav1.ObjectMeta{},
159-
Spec: AzureManagedClusterSpec{
160-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
161-
Host: "this-is-not-allowed",
162-
Port: 443,
163-
},
164-
},
165-
},
166-
wantErr: true,
167-
},
168-
{
169-
name: "ControlPlaneEndpoint update from zero values are allowed",
170-
oldAMC: &AzureManagedCluster{
171-
ObjectMeta: metav1.ObjectMeta{},
172-
Spec: AzureManagedClusterSpec{
173-
ControlPlaneEndpoint: clusterv1.APIEndpoint{},
174-
},
175-
},
176155
amc: &AzureManagedCluster{
177156
ObjectMeta: metav1.ObjectMeta{},
178157
Spec: AzureManagedClusterSpec{
@@ -211,26 +190,26 @@ func TestAzureManagedCluster_ValidateCreate(t *testing.T) {
211190
wantErr bool
212191
}{
213192
{
214-
name: "can't set Spec.ControlPlaneEndpoint.Host during create",
193+
name: "can set Spec.ControlPlaneEndpoint.Host during create (clusterctl move scenario)",
215194
amc: &AzureManagedCluster{
216195
Spec: AzureManagedClusterSpec{
217196
ControlPlaneEndpoint: clusterv1.APIEndpoint{
218197
Host: "my-host",
219198
},
220199
},
221200
},
222-
wantErr: true,
201+
wantErr: false,
223202
},
224203
{
225-
name: "can't set Spec.ControlPlaneEndpoint.Port during create",
204+
name: "can set Spec.ControlPlaneEndpoint.Port during create (clusterctl move scenario)",
226205
amc: &AzureManagedCluster{
227206
Spec: AzureManagedClusterSpec{
228207
ControlPlaneEndpoint: clusterv1.APIEndpoint{
229208
Port: 4443,
230209
},
231210
},
232211
},
233-
wantErr: true,
212+
wantErr: false,
234213
},
235214
}
236215
for _, tc := range tests {

‎api/v1beta1/azuremanagedcontrolplane_webhook.go‎

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,11 @@ import (
4141
)
4242

4343
var (
44-
kubeSemver = regexp.MustCompile(`^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
45-
controlPlaneEndpointErrorMessage = "can not be set by the user, will be set automatically by AKS after the cluster is Ready"
46-
rMaxNodeProvisionTime = regexp.MustCompile(`^(\d+)m$`)
47-
rScaleDownTime = regexp.MustCompile(`^(\d+)m$`)
48-
rScaleDownDelayAfterDelete = regexp.MustCompile(`^(\d+)s$`)
49-
rScanInterval = regexp.MustCompile(`^(\d+)s$`)
44+
kubeSemver = regexp.MustCompile(`^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
45+
rMaxNodeProvisionTime = regexp.MustCompile(`^(\d+)m$`)
46+
rScaleDownTime = regexp.MustCompile(`^(\d+)m$`)
47+
rScaleDownDelayAfterDelete = regexp.MustCompile(`^(\d+)s$`)
48+
rScanInterval = regexp.MustCompile(`^(\d+)s$`)
5049
)
5150

5251
// SetupAzureManagedControlPlaneWebhookWithManager sets up and registers the webhook with the manager.
@@ -116,19 +115,6 @@ func (mw *azureManagedControlPlaneWebhook) ValidateCreate(ctx context.Context, o
116115
)
117116
}
118117

119-
if m.Spec.ControlPlaneEndpoint.Host != "" {
120-
return field.Forbidden(
121-
field.NewPath("Spec", "ControlPlaneEndpoint", "Host"),
122-
controlPlaneEndpointErrorMessage,
123-
)
124-
}
125-
if m.Spec.ControlPlaneEndpoint.Port != 0 {
126-
return field.Forbidden(
127-
field.NewPath("Spec", "ControlPlaneEndpoint", "Port"),
128-
controlPlaneEndpointErrorMessage,
129-
)
130-
}
131-
132118
return m.Validate(mw.Client)
133119
}
134120

@@ -232,24 +218,6 @@ func (mw *azureManagedControlPlaneWebhook) ValidateUpdate(ctx context.Context, o
232218
}
233219
}
234220

235-
if old.Spec.ControlPlaneEndpoint.Host != "" {
236-
if err := webhookutils.ValidateImmutable(
237-
field.NewPath("Spec", "ControlPlaneEndpoint", "Host"),
238-
old.Spec.ControlPlaneEndpoint.Host,
239-
m.Spec.ControlPlaneEndpoint.Host); err != nil {
240-
allErrs = append(allErrs, err)
241-
}
242-
}
243-
244-
if old.Spec.ControlPlaneEndpoint.Port != 0 {
245-
if err := webhookutils.ValidateImmutable(
246-
field.NewPath("Spec", "ControlPlaneEndpoint", "Port"),
247-
old.Spec.ControlPlaneEndpoint.Port,
248-
m.Spec.ControlPlaneEndpoint.Port); err != nil {
249-
allErrs = append(allErrs, err)
250-
}
251-
}
252-
253221
// Consider removing this once moves out of preview
254222
// Updating outboundType after cluster creation (PREVIEW)
255223
// https://learn.microsoft.com/en-us/azure/aks/egress-outboundtype#updating-outboundtype-after-cluster-creation-preview

‎api/v1beta1/azuremanagedcontrolplane_webhook_test.go‎

Lines changed: 4 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
635635
errorLen: 1,
636636
},
637637
{
638-
name: "can't set Spec.ControlPlaneEndpoint.Host during create",
638+
name: "set Spec.ControlPlaneEndpoint.Host during create (clusterctl move scenario)",
639639
amcp: &AzureManagedControlPlane{
640640
Spec: AzureManagedControlPlaneSpec{
641641
ControlPlaneEndpoint: clusterv1.APIEndpoint{
@@ -652,10 +652,10 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
652652
},
653653
},
654654
},
655-
wantErr: true,
655+
wantErr: false,
656656
},
657657
{
658-
name: "can't set Spec.ControlPlaneEndpoint.Port during create",
658+
name: "can set Spec.ControlPlaneEndpoint.Port during create (clusterctl move scenario)",
659659
amcp: &AzureManagedControlPlane{
660660
Spec: AzureManagedControlPlaneSpec{
661661
ControlPlaneEndpoint: clusterv1.APIEndpoint{
@@ -672,7 +672,7 @@ func TestAzureManagedControlPlane_ValidateCreate(t *testing.T) {
672672
},
673673
},
674674
},
675-
wantErr: true,
675+
wantErr: false,
676676
},
677677
}
678678
for _, tc := range tests {
@@ -1231,81 +1231,6 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
12311231
},
12321232
wantErr: false,
12331233
},
1234-
{
1235-
name: "AzureManagedControlPlane ControlPlaneEndpoint.Port is immutable",
1236-
oldAMCP: &AzureManagedControlPlane{
1237-
ObjectMeta: metav1.ObjectMeta{
1238-
Name: "test-cluster",
1239-
},
1240-
Spec: AzureManagedControlPlaneSpec{
1241-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
1242-
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
1243-
Port: 443,
1244-
},
1245-
},
1246-
},
1247-
amcp: &AzureManagedControlPlane{
1248-
ObjectMeta: metav1.ObjectMeta{
1249-
Name: "test-cluster",
1250-
},
1251-
Spec: AzureManagedControlPlaneSpec{
1252-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
1253-
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
1254-
Port: 444,
1255-
},
1256-
},
1257-
},
1258-
wantErr: true,
1259-
},
1260-
{
1261-
name: "AzureManagedControlPlane ControlPlaneEndpoint.Host is immutable",
1262-
oldAMCP: &AzureManagedControlPlane{
1263-
ObjectMeta: metav1.ObjectMeta{
1264-
Name: "test-cluster",
1265-
},
1266-
Spec: AzureManagedControlPlaneSpec{
1267-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
1268-
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
1269-
Port: 443,
1270-
},
1271-
},
1272-
},
1273-
amcp: &AzureManagedControlPlane{
1274-
ObjectMeta: metav1.ObjectMeta{
1275-
Name: "test-cluster",
1276-
},
1277-
Spec: AzureManagedControlPlaneSpec{
1278-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
1279-
Host: "this-is-not-allowed",
1280-
Port: 443,
1281-
},
1282-
},
1283-
},
1284-
wantErr: true,
1285-
},
1286-
{
1287-
name: "ControlPlaneEndpoint update from zero values are allowed",
1288-
oldAMCP: &AzureManagedControlPlane{
1289-
ObjectMeta: metav1.ObjectMeta{
1290-
Name: "test-cluster",
1291-
},
1292-
Spec: AzureManagedControlPlaneSpec{
1293-
ControlPlaneEndpoint: clusterv1.APIEndpoint{},
1294-
},
1295-
},
1296-
amcp: &AzureManagedControlPlane{
1297-
ObjectMeta: metav1.ObjectMeta{
1298-
Name: "test-cluster",
1299-
},
1300-
Spec: AzureManagedControlPlaneSpec{
1301-
ControlPlaneEndpoint: clusterv1.APIEndpoint{
1302-
Host: "aks-8622-h4h26c44.hcp.eastus.azmk8s.io",
1303-
Port: 443,
1304-
},
1305-
},
1306-
},
1307-
wantErr: true,
1308-
},
13091234
{
13101235
name: "OutboundType update",
13111236
oldAMCP: &AzureManagedControlPlane{

‎azure/scope/managedcontrolplane.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -578,12 +578,8 @@ func (s *ManagedControlPlaneScope) GetAllAgentPoolSpecs() ([]azure.ResourceSpecG
578578

579579
// SetControlPlaneEndpoint sets a control plane endpoint.
580580
func (s *ManagedControlPlaneScope) SetControlPlaneEndpoint(endpoint clusterv1.APIEndpoint) {
581-
if s.ControlPlane.Spec.ControlPlaneEndpoint.Host == "" {
582-
s.ControlPlane.Spec.ControlPlaneEndpoint.Host = endpoint.Host
583-
}
584-
if s.ControlPlane.Spec.ControlPlaneEndpoint.Port == 0 {
585-
s.ControlPlane.Spec.ControlPlaneEndpoint.Port = endpoint.Port
586-
}
581+
s.ControlPlane.Spec.ControlPlaneEndpoint.Host = endpoint.Host
582+
s.ControlPlane.Spec.ControlPlaneEndpoint.Port = endpoint.Port
587583
}
588584

589585
// MakeEmptyKubeConfigSecret creates an empty secret object that is used for storing kubeconfig secret data.

‎controllers/azuremanagedcluster_controller.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,7 @@ func (amcr *AzureManagedClusterReconciler) Reconcile(ctx context.Context, req ct
161161
// Infrastructure must be ready before control plane. We should also enqueue
162162
// requests from control plane to infra cluster to keep control plane endpoint accurate.
163163
aksCluster.Status.Ready = true
164-
// We only expect to set the apiserver endpoint values once;
165-
// if we attempted to update existing ControlPlaneEndpoint values, they would be rejected via webhook enforcement.
166-
if aksCluster.Spec.ControlPlaneEndpoint.Host == "" {
167-
aksCluster.Spec.ControlPlaneEndpoint.Host = controlPlane.Spec.ControlPlaneEndpoint.Host
168-
}
169-
if aksCluster.Spec.ControlPlaneEndpoint.Port == 0 {
170-
aksCluster.Spec.ControlPlaneEndpoint.Port = controlPlane.Spec.ControlPlaneEndpoint.Port
171-
}
164+
aksCluster.Spec.ControlPlaneEndpoint = controlPlane.Spec.ControlPlaneEndpoint
172165

173166
if err := patchhelper.Patch(ctx, aksCluster); err != nil {
174167
return reconcile.Result{}, err

‎docs/book/src/topics/managedcluster.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ spec:
194194
sku: Standard_D2s_v4
195195
```
196196
197+
Please note that we don't declare a configuration for the apiserver endpoint. This configuration data will be populated automatically based on the data returned from AKS API during cluster create as `.spec.controlPlaneEndpoint.Host` and `.spec.controlPlaneEndpoint.Port` in both the `AzureManagedCluster` and `AzureManagedControlPlane` resources. Any user-provided data will be ignored and overwritten by data returned from the AKS API.
198+
197199
The main features for configuration are:
198200

199201
- [networkPolicy](https://docs.microsoft.com/en-us/azure/aks/concepts-network#network-policies)
@@ -717,7 +719,8 @@ Following is the list of immutable fields for managed clusters:
717719

718720
| CRD | jsonPath | Comment |
719721
|---------------------------|------------------------------|---------------------------|
720-
| AzureManagedControlPlane | .name | |
722+
| AzureManagedCluster | .spec.controlPlaneEndpoint | populated by the AKS API during cluster create |
723+
| AzureManagedControlPlane | .spec.controlPlaneEndpoint | populated by the AKS API during cluster create |
721724
| AzureManagedControlPlane | .spec.subscriptionID | |
722725
| AzureManagedControlPlane | .spec.resourceGroupName | |
723726
| AzureManagedControlPlane | .spec.nodeResourceGroupName | |

0 commit comments

Comments
 (0)