Skip to content

Commit 9c0af5b

Browse files
Add validations for AzureCluster Updates
1 parent 7487093 commit 9c0af5b

File tree

2 files changed

+80
-5
lines changed

2 files changed

+80
-5
lines changed

api/v1alpha3/azurecluster_webhook.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ limitations under the License.
1717
package v1alpha3
1818

1919
import (
20+
"reflect"
21+
22+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2023
"k8s.io/apimachinery/pkg/runtime"
24+
"k8s.io/apimachinery/pkg/util/validation/field"
2125
ctrl "sigs.k8s.io/controller-runtime"
2226
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
2327
"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -56,8 +60,35 @@ func (c *AzureCluster) ValidateCreate() error {
5660
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
5761
func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
5862
clusterlog.Info("validate update", "name", c.Name)
63+
var allErrs field.ErrorList
5964
old := oldRaw.(*AzureCluster)
60-
return c.validateCluster(old)
65+
66+
if !reflect.DeepEqual(c.Spec.ResourceGroup, old.Spec.ResourceGroup) {
67+
allErrs = append(allErrs,
68+
field.Invalid(field.NewPath("spec", "ResourceGroup"),
69+
c.Spec.ResourceGroup, "field is immutable"),
70+
)
71+
}
72+
73+
if !reflect.DeepEqual(c.Spec.SubscriptionID, old.Spec.SubscriptionID) {
74+
allErrs = append(allErrs,
75+
field.Invalid(field.NewPath("spec", "SubscriptionID"),
76+
c.Spec.SubscriptionID, "field is immutable"),
77+
)
78+
}
79+
80+
if !reflect.DeepEqual(c.Spec.Location, old.Spec.Location) {
81+
allErrs = append(allErrs,
82+
field.Invalid(field.NewPath("spec", "Location"),
83+
c.Spec.Location, "field is immutable"),
84+
)
85+
}
86+
87+
if len(allErrs) == 0 {
88+
return c.validateCluster(old)
89+
}
90+
91+
return apierrors.NewInvalid(GroupVersion.WithKind("AzureCluster").GroupKind(), c.Name, allErrs)
6192
}
6293

6394
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

api/v1alpha3/azurecluster_webhook_test.go

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,10 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
100100
g := NewWithT(t)
101101

102102
tests := []struct {
103-
name string
104-
cluster *AzureCluster
105-
wantErr bool
103+
name string
104+
oldCluster *AzureCluster
105+
cluster *AzureCluster
106+
wantErr bool
106107
}{
107108
{
108109
name: "azurecluster with pre-existing vnet - valid spec",
@@ -157,10 +158,53 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
157158
}(),
158159
wantErr: true,
159160
},
161+
{
162+
name: "azurecluster resource group is immutable",
163+
oldCluster: &AzureCluster{
164+
Spec: AzureClusterSpec{
165+
ResourceGroup: "demoResourceGroup",
166+
},
167+
},
168+
cluster: &AzureCluster{
169+
Spec: AzureClusterSpec{
170+
ResourceGroup: "demoResourceGroup-2",
171+
},
172+
},
173+
wantErr: true,
174+
},
175+
{
176+
name: "azurecluster subscription ID is immutable",
177+
oldCluster: &AzureCluster{
178+
Spec: AzureClusterSpec{
179+
SubscriptionID: "212ec1q8",
180+
},
181+
},
182+
cluster: &AzureCluster{
183+
Spec: AzureClusterSpec{
184+
SubscriptionID: "212ec1q9",
185+
},
186+
},
187+
wantErr: true,
188+
},
189+
{
190+
name: "azurecluster location is immutable",
191+
oldCluster: &AzureCluster{
192+
Spec: AzureClusterSpec{
193+
Location: "North Europe",
194+
},
195+
},
196+
cluster: &AzureCluster{
197+
Spec: AzureClusterSpec{
198+
Location: "West Europe",
199+
},
200+
},
201+
wantErr: true,
202+
},
160203
}
161204
for _, tc := range tests {
162205
t.Run(tc.name, func(t *testing.T) {
163-
err := tc.cluster.ValidateUpdate(createValidCluster())
206+
t.Parallel()
207+
err := tc.cluster.ValidateUpdate(tc.oldCluster)
164208
if tc.wantErr {
165209
g.Expect(err).To(HaveOccurred())
166210
} else {

0 commit comments

Comments
 (0)