Set Defaults functionality #1654

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

Merged

mjudeikis merged 9 commits into Azure:master from mjudeikis:set.defaults

Aug 11, 2021

.sha256sum

-Original file line number
+Diff line change
@@ -1,2 +1,2 @@
     d2e11a7924d0cbb70672fb0dd6b1a387ccaec8b97a6968adf5a1516d325374eb  swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/stable/2020-04-30/redhatopenshift.json
-f91bafd1bbebd4e348bb754e197bd0bdf4c0b06debdb47c78e46eb8ddede5f  swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/preview/2021-09-01-preview/redhatopenshift.json
+f87c84f68049c9da77f339e5aaae0e6ec86831992a31f6d6a5f997cced3ef  swagger/redhatopenshift/resource-manager/Microsoft.RedHatOpenShift/preview/2021-09-01-preview/redhatopenshift.json

pkg/api/defaults.go

-Original file line number
+Diff line change
@@ -0,0 +1,29 @@
+    package api
+    // Copyright (c) Microsoft Corporation.
+    // Licensed under the Apache License 2.0.
+    // SetDefaults sets the default values for older api version
+    // when interacting with newer api versions. This together with
+    // database migration will make sure we have right values in the cluster documents
+    // when moving between old and new versions
+    func SetDefaults(doc *OpenShiftClusterDocument) {
+    	if doc.OpenShiftCluster != nil {
+    		// SDNProvider was introduced in 2021-09-01-preview
+    		if doc.OpenShiftCluster.Properties.NetworkProfile.SDNProvider == "" {
+    			doc.OpenShiftCluster.Properties.NetworkProfile.SDNProvider = SDNProviderOpenShiftSDN
+    		}
+    		// EncryptionAtHost was introduced in 2021-09-01-preview.
+    		// It can't be changed post cluster creation
+    		if doc.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost == "" {
+    			doc.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost = EncryptionAtHostDisabled
+    		}
+    		for i, wp := range doc.OpenShiftCluster.Properties.WorkerProfiles {
+    			if wp.EncryptionAtHost == "" {
+    				doc.OpenShiftCluster.Properties.WorkerProfiles[i].EncryptionAtHost = EncryptionAtHostDisabled
+    			}
+    		}
+    	}
+    }

pkg/api/defaults_test.go

-Original file line number
+Diff line change
@@ -0,0 +1,100 @@
+    package api
+    // Copyright (c) Microsoft Corporation.
+    // Licensed under the Apache License 2.0.
+    import (
+    	"fmt"
+    	"reflect"
+    	"testing"
+    )
+    func validOpenShiftClusterDocument() *OpenShiftClusterDocument {
+    	doc := OpenShiftClusterDocument{
+    		OpenShiftCluster: &OpenShiftCluster{
+    			Properties: OpenShiftClusterProperties{
+    				NetworkProfile: NetworkProfile{
+    					SDNProvider: SDNProviderOpenShiftSDN,
+    				},
+    				MasterProfile: MasterProfile{
+    					EncryptionAtHost: EncryptionAtHostDisabled,
+    				},
+    				WorkerProfiles: []WorkerProfile{
+    					{
+    						EncryptionAtHost: EncryptionAtHostDisabled,
+    					},
+    				},
+    			},
+    		},
+    	}
+    	return &doc
+    }
+    func TestSetDefaults(t *testing.T) {
+    	for _, tt := range []struct {
+    		name  string
+    		want  func() *OpenShiftClusterDocument
+    		input func(doc *OpenShiftClusterDocument)
+    	}{
+    		{
+    			name: "no defaults needed",
+    			want: func() *OpenShiftClusterDocument {
+    				return validOpenShiftClusterDocument()
+    			},
+    		},
+    		{
+    			name: "default SDN",
+    			want: func() *OpenShiftClusterDocument {
+    				return validOpenShiftClusterDocument()
+    			},
+    			input: func(base *OpenShiftClusterDocument) {
+    				base.OpenShiftCluster.Properties.NetworkProfile.SDNProvider = ""
+    			},
+    		},
+    		{
+    			name: "preserve SDN",
+    			want: func() *OpenShiftClusterDocument {
+    				doc := validOpenShiftClusterDocument()
+    				doc.OpenShiftCluster.Properties.NetworkProfile.SDNProvider = SDNProviderOVNKubernetes
+    				return doc
+    			},
+    			input: func(base *OpenShiftClusterDocument) {
+    				base.OpenShiftCluster.Properties.NetworkProfile.SDNProvider = SDNProviderOVNKubernetes
+    			},
+    		},
+    		{
+    			name: "default encryption at host",
+    			want: func() *OpenShiftClusterDocument {
+    				return validOpenShiftClusterDocument()
+    			},
+    			input: func(base *OpenShiftClusterDocument) {
+    				base.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost = ""
+    			},
+    		},
+    		{
+    			name: "preserve encryption at host",
+    			want: func() *OpenShiftClusterDocument {
+    				doc := validOpenShiftClusterDocument()
+    				doc.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost = EncryptionAtHostEnabled
+    				return doc
+    			},
+    			input: func(base *OpenShiftClusterDocument) {
+    				base.OpenShiftCluster.Properties.MasterProfile.EncryptionAtHost = EncryptionAtHostEnabled
+    			},
+    		},
+    	} {
+    		t.Run(tt.name, func(t *testing.T) {
+    			doc := validOpenShiftClusterDocument()
+    			want := tt.want()
+    			if tt.input != nil {
+    				tt.input(doc)
+    			}
+    			SetDefaults(doc)
+    			if !reflect.DeepEqual(&doc, &want) {
+    				t.Error(fmt.Errorf("\n%+v\n !=\n%+v", doc, want)) // can't use cmp due to cycle imports
+    			}
+    		})
+    	}
+    }

pkg/api/v20210901preview/openshiftcluster_validatestatic.go

-Original file line number
+Diff line change
@@ Expand Up @@
     	if sr.SubscriptionID != sv.r.SubscriptionID {
     		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".subnetId", "The provided master VM subnet '%s' is invalid: must be in same subscription as cluster.", mp.SubnetID)
     	}
+    	switch mp.EncryptionAtHost {
+    	case EncryptionAtHostDisabled, EncryptionAtHostEnabled:
+    	default:
+    		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".encryptionAtHost", "The provided value '%s' is invalid.", mp.EncryptionAtHost)
+    	}
     	if mp.DiskEncryptionSetID != "" {
     		if !validate.RxDiskEncryptionSetID.MatchString(mp.DiskEncryptionSetID) {
     			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".diskEncryptionSetId", "The provided master disk encryption set '%s' is invalid.", mp.DiskEncryptionSetID)
@@ Expand Down Expand Up @@
     	if !validate.RxSubnetID.MatchString(wp.SubnetID) {
     		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".subnetId", "The provided worker VM subnet '%s' is invalid.", wp.SubnetID)
     	}
+    	switch mp.EncryptionAtHost {
+    	case EncryptionAtHostDisabled, EncryptionAtHostEnabled:
+    	default:
+    		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, path+".encryptionAtHost", "The provided value '%s' is invalid.", mp.EncryptionAtHost)
+    	}
     	workerVnetID, _, err := subnet.Split(wp.SubnetID)
     	if err != nil {
     		return err
@@ Expand Down @@

pkg/api/v20210901preview/openshiftcluster_validatestatic_test.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -81,16 +81,18 @@ func validOpenShiftCluster() *OpenShiftCluster { @@
     				SDNProvider: SDNProviderOVNKubernetes,
     			},
     			MasterProfile: MasterProfile{
-    				VMSize:   VMSizeStandardD8sV3,
-    				SubnetID: fmt.Sprintf("/subscriptions/%s/resourceGroups/vnet/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/master", subscriptionID),
+    				VMSize:           VMSizeStandardD8sV3,
+    				EncryptionAtHost: EncryptionAtHostDisabled,
+    				SubnetID:         fmt.Sprintf("/subscriptions/%s/resourceGroups/vnet/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/master", subscriptionID),
     			},
     			WorkerProfiles: []WorkerProfile{
     				{
-    					Name:       "worker",
-    					VMSize:     VMSizeStandardD4sV3,
-    					DiskSizeGB: 128,
-    					SubnetID:   fmt.Sprintf("/subscriptions/%s/resourceGroups/vnet/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/worker", subscriptionID),
-    					Count:      3,
+    					Name:             "worker",
+    					VMSize:           VMSizeStandardD4sV3,
+    					EncryptionAtHost: EncryptionAtHostDisabled,
+    					DiskSizeGB:       128,
+    					SubnetID:         fmt.Sprintf("/subscriptions/%s/resourceGroups/vnet/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/worker", subscriptionID),
+    					Count:            3,
     				},
     			},
     			APIServerProfile: APIServerProfile{
@@ Expand Down Expand Up @@
     			},
     			wantErr: "400: InvalidParameter: properties.masterProfile.diskEncryptionSetId: The provided master disk encryption set '/subscriptions/7a3036d1-60a1-4605-8a41-44955e050804/resourceGroups/fakeRG/providers/Microsoft.Compute/diskEncryptionSets/fakeDES1' is invalid: must be in same subscription as cluster.",
     		},
+    		{
+    			name: "encryption at host invalid",
+    			modify: func(oc *OpenShiftCluster) {
+    				oc.Properties.MasterProfile.EncryptionAtHost = "Banana"
+    			},
+    			wantErr: "400: InvalidParameter: properties.masterProfile.encryptionAtHost: The provided value 'Banana' is invalid.",
+    		},
+    		{
+    			name: "encryption at host empty",
+    			modify: func(oc *OpenShiftCluster) {
+    				oc.Properties.MasterProfile.EncryptionAtHost = ""
+    			},
+    			wantErr: "400: InvalidParameter: properties.masterProfile.encryptionAtHost: The provided value '' is invalid.",
+    		},
     	}
     	createTests := []*validateTest{
@@ Expand Down @@

pkg/client/services/redhatopenshift/mgmt/2020-04-30/redhatopenshift/openshiftclusters.go

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

...ent/services/redhatopenshift/mgmt/2021-09-01-preview/redhatopenshift/openshiftclusters.go

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

.../redhatopenshift/mgmt/2021-09-01-preview/redhatopenshift/redhatopenshiftapi/interfaces.go

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

pkg/cluster/defaults.go

-Original file line number
+Diff line change
@@ -0,0 +1,25 @@
+    package cluster
+    // Copyright (c) Microsoft Corporation.
+    // Licensed under the Apache License 2.0.
+    import (
+    	"context"
+    	"github.com/Azure/ARO-RP/pkg/api"
+    )
+    // ensureDefaults will ensure cluster documents has all default values
+    // for new api versions
+    func (m *manager) ensureDefaults(ctx context.Context) error {
+    	var err error
+    	m.doc, err = m.db.PatchWithLease(ctx, m.doc.Key, func(doc *api.OpenShiftClusterDocument) error {
+    		api.SetDefaults(doc)
+    		return nil
+    	})
+    	if err != nil {
+    		m.log.Print(err)
+    		return err
+    	}
+    	return nil
+    }

pkg/cluster/install.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -31,6 +31,7 @@ func (m *manager) AdminUpdate(ctx context.Context) error { @@
     	steps := []steps.Step{
     		steps.Action(m.initializeKubernetesClients), // must be first
     		steps.Action(m.fixupClusterSPObjectID),
+    		steps.Action(m.ensureDefaults),
     		steps.AuthorizationRefreshingAction(m.fpAuthorizer, steps.Action(m.ensureResourceGroup)), // re-create RP RBAC if needed after tenant migration
     		steps.Action(m.createOrUpdateDenyAssignment),
     		steps.Action(m.startVMs),
@@ Expand Down @@

pkg/frontend/openshiftcluster_putorpatch.go

-Original file line number
+Diff line change
@@ Expand Up @@
     	// is not provided in the header must be preserved
     	f.systemDataEnricher(doc, systemData)
+    	// SetDefaults will set defaults on cluster document
+    	api.SetDefaults(doc)
     	if isCreate {
     		// on create, make the cluster resourcegroup ID lower case to work
     		// around LB/PLS bug
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set Defaults functionality #1654

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!