Skip to content

Commit 210f14a

Browse files
anirudhAgniRedhatanirudhAgniRedhat
committed
Revert "Added immutability validations"
This reverts commit 41c1c37.
1 parent 41c1c37 commit 210f14a

File tree

4 files changed

+33
-220
lines changed

4 files changed

+33
-220
lines changed

‎pkg/webhooks/machine_webhook.go

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ func getDNS() (*osconfigv1.DNS, error) {
315315
return dns, nil
316316
}
317317

318-
type machineAdmissionFn func(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList)
318+
type machineAdmissionFn func(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList)
319319

320320
type admissionConfig struct {
321321
clusterID string
@@ -396,7 +396,7 @@ func getMachineValidatorOperation(platform osconfigv1.PlatformType) machineAdmis
396396
return validateNutanix
397397
default:
398398
// just no-op
399-
return func(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
399+
return func(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
400400
return true, []string{}, nil
401401
}
402402
}
@@ -441,7 +441,7 @@ func getMachineDefaulterOperation(platformStatus *osconfigv1.PlatformStatus) mac
441441
return defaultNutanix
442442
default:
443443
// just no-op
444-
return func(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
444+
return func(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
445445
return true, []string{}, nil
446446
}
447447
}
@@ -462,7 +462,7 @@ func (h *machineValidatorHandler) validateMachine(m, oldM *machinev1beta1.Machin
462462

463463
errs := validateMachineLifecycleHooks(m, oldM)
464464

465-
ok, warnings, opErrs := h.webhookOperations(m, oldM, h.admissionConfig)
465+
ok, warnings, opErrs := h.webhookOperations(m, h.admissionConfig)
466466
if !ok {
467467
errs = append(errs, opErrs...)
468468
}
@@ -549,7 +549,7 @@ func (h *machineDefaulterHandler) Default(ctx context.Context, obj runtime.Objec
549549
m.Labels[machinev1beta1.MachineClusterIDLabel] = h.clusterID
550550
}
551551

552-
ok, _, errs := h.webhookOperations(m, nil, h.admissionConfig)
552+
ok, _, errs := h.webhookOperations(m, h.admissionConfig)
553553
if !ok {
554554
return errs.ToAggregate()
555555
}
@@ -561,7 +561,7 @@ type awsDefaulter struct {
561561
region string
562562
}
563563

564-
func (a awsDefaulter) defaultAWS(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
564+
func (a awsDefaulter) defaultAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
565565
klog.V(3).Infof("Defaulting AWS providerSpec")
566566

567567
var errs field.ErrorList
@@ -633,7 +633,7 @@ func validateUnknownFields(m *machinev1beta1.Machine, providerSpec interface{})
633633
return nil
634634
}
635635

636-
func validateAWS(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
636+
func validateAWS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
637637
klog.V(3).Infof("Validating AWS providerSpec")
638638

639639
var errs field.ErrorList
@@ -803,7 +803,7 @@ func getDuplicatedTags(tagSpecs []machinev1beta1.TagSpecification) []string {
803803
return duplicatedTags
804804
}
805805

806-
func defaultAzure(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
806+
func defaultAzure(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
807807
klog.V(3).Infof("Defaulting Azure providerSpec")
808808

809809
var errs field.ErrorList
@@ -864,7 +864,7 @@ func defaultAzure(m, mOld *machinev1beta1.Machine, config *admissionConfig) (boo
864864
return true, warnings, nil
865865
}
866866

867-
func validateAzure(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
867+
func validateAzure(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
868868
klog.V(3).Infof("Validating Azure providerSpec")
869869

870870
var errs field.ErrorList
@@ -931,18 +931,6 @@ func validateAzure(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bo
931931
fmt.Sprintf("osDisk.diskSettings.ephemeralStorageLocation can either be omitted or set to %s", azureEphemeralStorageLocationLocal)))
932932
}
933933

934-
if mOld != nil {
935-
oldProviderSpec := new(machinev1beta1.AzureMachineProviderSpec)
936-
if err := unmarshalInto(mOld, oldProviderSpec); err != nil {
937-
errs = append(errs, err)
938-
return false, warnings, errs
939-
}
940-
if providerSpec.CapacityReservationGroupID != "" && !validateAzureImmutabilityForCapacityReservationGroupID(oldProviderSpec.CapacityReservationGroupID, providerSpec.CapacityReservationGroupID) {
941-
errs = append(errs, field.Invalid(field.NewPath("providerSpec", "capacityReservationGroupID"),
942-
providerSpec.CapacityReservationGroupID, "capacityReservationGroupID is immutable"))
943-
}
944-
}
945-
946934
if providerSpec.CapacityReservationGroupID != "" {
947935
err := validateAzureCapacityReservationGroupID(providerSpec.CapacityReservationGroupID)
948936
if err != nil {
@@ -1043,7 +1031,7 @@ func validateAzureDiagnostics(diagnosticsSpec machinev1beta1.AzureDiagnostics, p
10431031
return errs
10441032
}
10451033

1046-
func defaultGCP(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1034+
func defaultGCP(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
10471035
klog.V(3).Infof("Defaulting GCP providerSpec")
10481036

10491037
var errs field.ErrorList
@@ -1131,7 +1119,7 @@ func defaultGCPDisks(disks []*machinev1beta1.GCPDisk, clusterID string) []*machi
11311119
return disks
11321120
}
11331121

1134-
func validateGCP(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1122+
func validateGCP(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
11351123
klog.V(3).Infof("Validating GCP providerSpec")
11361124

11371125
var errs field.ErrorList
@@ -1366,7 +1354,7 @@ func validateGCPServiceAccounts(serviceAccounts []machinev1beta1.GCPServiceAccou
13661354
return errs
13671355
}
13681356

1369-
func defaultVSphere(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1357+
func defaultVSphere(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
13701358
klog.V(3).Infof("Defaulting vSphere providerSpec")
13711359

13721360
var errs field.ErrorList
@@ -1398,7 +1386,7 @@ func defaultVSphere(m, mOld *machinev1beta1.Machine, config *admissionConfig) (b
13981386
return true, warnings, nil
13991387
}
14001388

1401-
func validateVSphere(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1389+
func validateVSphere(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
14021390
klog.V(3).Infof("Validating vSphere providerSpec")
14031391

14041392
var errs field.ErrorList
@@ -1514,7 +1502,7 @@ func validateVSphereNetwork(network machinev1beta1.NetworkSpec, parentPath *fiel
15141502
return errs
15151503
}
15161504

1517-
func defaultNutanix(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1505+
func defaultNutanix(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
15181506
klog.V(3).Infof("Defaulting nutanix providerSpec")
15191507

15201508
var errs field.ErrorList
@@ -1546,7 +1534,7 @@ func defaultNutanix(m, mOld *machinev1beta1.Machine, config *admissionConfig) (b
15461534
return true, warnings, nil
15471535
}
15481536

1549-
func validateNutanix(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1537+
func validateNutanix(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
15501538
klog.V(3).Infof("Validating nutanix providerSpec")
15511539

15521540
var errs field.ErrorList
@@ -1855,7 +1843,7 @@ func validateAzureDataDisks(machineName string, spec *machinev1beta1.AzureMachin
18551843
return errs
18561844
}
18571845

1858-
func defaultPowerVS(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1846+
func defaultPowerVS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
18591847
klog.V(3).Infof("Defaulting PowerVS providerSpec")
18601848

18611849
var errs field.ErrorList
@@ -1902,7 +1890,7 @@ func defaultPowerVS(m, mOld *machinev1beta1.Machine, config *admissionConfig) (b
19021890
return true, warnings, nil
19031891
}
19041892

1905-
func validatePowerVS(m, mOld *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
1893+
func validatePowerVS(m *machinev1beta1.Machine, config *admissionConfig) (bool, []string, field.ErrorList) {
19061894
klog.V(3).Infof("Validating PowerVS providerSpec")
19071895

19081896
var errs field.ErrorList
@@ -2081,10 +2069,6 @@ func validateGVK(gvk schema.GroupVersionKind, platform osconfigv1.PlatformType)
20812069
}
20822070
}
20832071

2084-
func validateAzureImmutabilityForCapacityReservationGroupID(oldID string, newID string) bool {
2085-
return oldID == newID
2086-
}
2087-
20882072
func validateAzureCapacityReservationGroupID(capacityReservationGroupID string) error {
20892073
id := strings.TrimPrefix(capacityReservationGroupID, azureProviderIDPrefix)
20902074
err := parseAzureResourceID(id)

‎pkg/webhooks/machine_webhook_test.go

Lines changed: 12 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,46 +1572,6 @@ func TestMachineUpdate(t *testing.T) {
15721572
},
15731573
expectedError: "providerSpec.osDisk.cachingType: Invalid value: \"\": Instances using an ephemeral OS disk support only Readonly caching",
15741574
},
1575-
{
1576-
name: "with an Azure ProviderSpec, changing capacityReservationGroupID to another ID",
1577-
platformType: osconfigv1.AzurePlatformType,
1578-
clusterID: azureClusterID,
1579-
baseProviderSpecValue: func() *kruntime.RawExtension {
1580-
defaultSpec := defaultAzureProviderSpec.DeepCopy()
1581-
defaultSpec.CapacityReservationGroupID = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"
1582-
return &kruntime.RawExtension{
1583-
Object: defaultSpec,
1584-
}
1585-
}(),
1586-
updatedProviderSpecValue: func() *kruntime.RawExtension {
1587-
object := defaultAzureProviderSpec.DeepCopy()
1588-
object.CapacityReservationGroupID = "/subscriptions/00000000-0000-0000-0000-000000000001/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"
1589-
return &kruntime.RawExtension{
1590-
Object: object,
1591-
}
1592-
},
1593-
expectedError: "admission webhook \"validation.machine.machine.openshift.io\" denied the request: providerSpec.capacityReservationGroupID: Invalid value: \"/subscriptions/00000000-0000-0000-0000-000000000001/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup\": capacityReservationGroupID is immutable",
1594-
},
1595-
{
1596-
name: "with an Azure ProviderSpec, changing capacityReservationGroupID to '' ",
1597-
platformType: osconfigv1.AzurePlatformType,
1598-
clusterID: azureClusterID,
1599-
baseProviderSpecValue: func() *kruntime.RawExtension {
1600-
defaultSpec := defaultAzureProviderSpec.DeepCopy()
1601-
defaultSpec.CapacityReservationGroupID = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup"
1602-
return &kruntime.RawExtension{
1603-
Object: defaultSpec,
1604-
}
1605-
}(),
1606-
updatedProviderSpecValue: func() *kruntime.RawExtension {
1607-
object := defaultAzureProviderSpec.DeepCopy()
1608-
object.CapacityReservationGroupID = ""
1609-
return &kruntime.RawExtension{
1610-
Object: object,
1611-
}
1612-
},
1613-
expectedError: "",
1614-
},
16151575
{
16161576
name: "with a valid GCP ProviderSpec",
16171577
platformType: osconfigv1.GCPPlatformType,
@@ -2384,7 +2344,7 @@ func TestValidateAWSProviderSpec(t *testing.T) {
23842344
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: tc.overrideRawBytes}
23852345
}
23862346

2387-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
2347+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
23882348
if ok != tc.expectedOk {
23892349
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
23902350
}
@@ -2460,7 +2420,7 @@ func TestDefaultAWSProviderSpec(t *testing.T) {
24602420
}
24612421
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
24622422

2463-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
2423+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
24642424
if ok != tc.expectedOk {
24652425
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
24662426
}
@@ -3043,7 +3003,7 @@ func TestValidateAzureProviderSpec(t *testing.T) {
30433003
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: tc.overrideRawBytes}
30443004
}
30453005

3046-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
3006+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
30473007
if ok != tc.expectedOk {
30483008
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
30493009
}
@@ -3199,7 +3159,7 @@ func TestDefaultAzureProviderSpec(t *testing.T) {
31993159
}
32003160
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
32013161

3202-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
3162+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
32033163
if ok != tc.expectedOk {
32043164
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
32053165
}
@@ -3739,7 +3699,7 @@ func TestValidateGCPProviderSpec(t *testing.T) {
37393699
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: tc.overrideRawBytes}
37403700
}
37413701

3742-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
3702+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
37433703
if ok != tc.expectedOk {
37443704
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
37453705
}
@@ -3878,7 +3838,7 @@ func TestDefaultGCPProviderSpec(t *testing.T) {
38783838
}
38793839
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
38803840

3881-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
3841+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
38823842
if ok != tc.expectedOk {
38833843
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
38843844
}
@@ -4176,7 +4136,7 @@ func TestValidateVSphereProviderSpec(t *testing.T) {
41764136
}
41774137
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
41784138

4179-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
4139+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
41804140
if ok != tc.expectedOk {
41814141
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
41824142
}
@@ -4241,7 +4201,7 @@ func TestDefaultVSphereProviderSpec(t *testing.T) {
42414201
}
42424202
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
42434203

4244-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
4204+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
42454205
if ok != tc.expectedOk {
42464206
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
42474207
}
@@ -4314,7 +4274,7 @@ func TestDefaultNutanixProviderSpec(t *testing.T) {
43144274
}
43154275
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
43164276

4317-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
4277+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
43184278
if ok != tc.expectedOk {
43194279
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
43204280
}
@@ -4835,7 +4795,7 @@ func TestValidatePowerVSProviderSpec(t *testing.T) {
48354795
}
48364796
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
48374797

4838-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
4798+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
48394799
if ok != tc.expectedOk {
48404800
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
48414801
}
@@ -4917,7 +4877,7 @@ func TestDefaultPowerVSProviderSpec(t *testing.T) {
49174877
}
49184878
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
49194879

4920-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
4880+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
49214881
if ok != tc.expectedOk {
49224882
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
49234883
}
@@ -5101,7 +5061,7 @@ func TestValidateNutanixProviderSpec(t *testing.T) {
51015061
}
51025062
m.Spec.ProviderSpec.Value = &kruntime.RawExtension{Raw: rawBytes}
51035063

5104-
ok, warnings, webhookErr := h.webhookOperations(m, nil, h.admissionConfig)
5064+
ok, warnings, webhookErr := h.webhookOperations(m, h.admissionConfig)
51055065
if ok != tc.expectedOk {
51065066
t.Errorf("expected: %v, got: %v", tc.expectedOk, ok)
51075067
}
@@ -5163,36 +5123,3 @@ func TestValidateAzureCapacityReservationGroupID(t *testing.T) {
51635123
})
51645124
}
51655125
}
5166-
5167-
func TestValidateImmutabilityForCapacityReservationGroupID(t *testing.T) {
5168-
testCases := []struct {
5169-
name string
5170-
oldID string
5171-
newID string
5172-
expect bool
5173-
}{
5174-
{
5175-
name: "validation for immutability of capacityReservationGroupID should return true if oldID and new ID are same",
5176-
oldID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
5177-
newID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
5178-
expect: true,
5179-
},
5180-
{
5181-
name: "validation for immutability of capacityReservationGroupID should return false if oldID and new ID are not same",
5182-
oldID: "subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
5183-
newID: "subscriptions/00000000-0000-0000-0000-000000000001/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/capacityReservationGroups/myCapacityReservationGroup",
5184-
expect: false,
5185-
},
5186-
}
5187-
5188-
for _, tc := range testCases {
5189-
t.Run(tc.name, func(t *testing.T) {
5190-
_ = NewWithT(t)
5191-
isValid := validateAzureImmutabilityForCapacityReservationGroupID(tc.oldID, tc.newID)
5192-
if isValid != tc.expect {
5193-
t.Errorf("expected: %v, got: %v", tc.expect, isValid)
5194-
}
5195-
5196-
})
5197-
}
5198-
}

0 commit comments

Comments
 (0)