From 3ee64eee6b8788845e436e53ad3182966a104e84 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Tue, 3 Sep 2024 16:16:42 -0700 Subject: [PATCH] Updates ingress configuration to loosely typed config object --- cli/azd/pkg/config/config.go | 33 +++++++ cli/azd/pkg/containerapps/container_app.go | 60 ++++--------- .../pkg/containerapps/container_app_test.go | 89 +++++++++++++++++++ .../test/mocks/mockazsdk/container_apps.go | 30 +++++++ cli/azd/test/mocks/util.go | 12 +++ 5 files changed, 183 insertions(+), 41 deletions(-) diff --git a/cli/azd/pkg/config/config.go b/cli/azd/pkg/config/config.go index aae0b43895c..3d96a17cac5 100644 --- a/cli/azd/pkg/config/config.go +++ b/cli/azd/pkg/config/config.go @@ -29,12 +29,23 @@ type Config interface { Raw() map[string]any // similar to Raw() but it will resolve any vault references ResolvedRaw() map[string]any + // Get retrieves the value stored at the specified path Get(path string) (any, bool) + // GetString retrieves the value stored at the specified path as a string GetString(path string) (string, bool) + // GetSection retrieves the value stored at the specified path and unmarshals it into the provided section GetSection(path string, section any) (bool, error) + // GetMap retrieves the map stored at the specified path + GetMap(path string) (map[string]any, bool) + // GetSlice retrieves the slice stored at the specified path + GetSlice(path string) ([]any, bool) + // Set stores the value at the specified path Set(path string, value any) error + // SetSecret stores the secrets at the specified path within a local user vault SetSecret(path string, value string) error + // Unset removes the value stored at the specified path Unset(path string) error + // IsEmpty returns a value indicating whether the configuration is empty IsEmpty() bool } @@ -230,6 +241,28 @@ func (c *config) Get(path string) (any, bool) { return nil, false } +// GetMap retrieves the map stored at the specified path +func (c *config) GetMap(path string) (map[string]any, bool) { + value, ok := c.Get(path) + if !ok { + return nil, false + } + + node, ok := value.(map[string]any) + return node, ok +} + +// GetSlice retrieves the slice stored at the specified path +func (c *config) GetSlice(path string) ([]any, bool) { + value, ok := c.Get(path) + if !ok { + return nil, false + } + + node, ok := value.([]any) + return node, ok +} + // Gets the value stored at the specified location as a string func (c *config) GetString(path string) (string, bool) { value, ok := c.Get(path) diff --git a/cli/azd/pkg/containerapps/container_app.go b/cli/azd/pkg/containerapps/container_app.go index 3b11ddbf6cd..68bfa90077a 100644 --- a/cli/azd/pkg/containerapps/container_app.go +++ b/cli/azd/pkg/containerapps/container_app.go @@ -30,7 +30,6 @@ const ( pathTemplateContainers = "properties.template.containers" pathConfigurationActiveRevisionsMode = "properties.configuration.activeRevisionsMode" pathConfigurationSecrets = "properties.configuration.secrets" - pathConfigurationIngress = "properties.configuration.ingress" pathConfigurationIngressTraffic = "properties.configuration.ingress.traffic" pathConfigurationIngressFqdn = "properties.configuration.ingress.fqdn" pathConfigurationIngressCustomDomains = "properties.configuration.ingress.customDomains" @@ -149,40 +148,23 @@ func (cas *containerAppService) persistSettings( log.Printf("failed getting current aca settings: %v. No settings will be persisted.", err) } - var ingress *armappcontainers.Ingress - if has, err := aca.GetSection(pathConfigurationIngress, &ingress); !has || err != nil { - return obj, err - } + objConfig := config.NewConfig(obj) - if shouldPersistDomains && - ingress.CustomDomains != nil { - acaAsConfig := config.NewConfig(obj) - customDomainsJson, err := convert.ToJsonArray(ingress.CustomDomains) - if err != nil { - return nil, fmt.Errorf("converting custom domains to JSON: %w", err) + if shouldPersistDomains { + customDomains, ok := aca.GetSlice(pathConfigurationIngressCustomDomains) + if ok { + objConfig.Set(pathConfigurationIngressCustomDomains, customDomains) } - - if err := acaAsConfig.Set(pathConfigurationIngressCustomDomains, customDomainsJson); err != nil { - return nil, fmt.Errorf("failed to persist custom domains: %w", err) - } - obj = acaAsConfig.Raw() } - if shouldPersistIngressSessionAffinity && - ingress.StickySessions != nil { - acaAsConfig := config.NewConfig(obj) - stickySessionsJson, err := convert.ToJsonArray(ingress.StickySessions) - if err != nil { - return nil, fmt.Errorf("converting sticky sessions to JSON: %w", err) - } - - if err := acaAsConfig.Set(pathConfigurationIngressStickySessions, stickySessionsJson); err != nil { - return nil, fmt.Errorf("failed to persist session affinity: %w", err) + if shouldPersistIngressSessionAffinity { + stickySessions, has := aca.Get(pathConfigurationIngressStickySessions) + if has { + objConfig.Set(pathConfigurationIngressStickySessions, stickySessions) } - obj = acaAsConfig.Raw() } - return obj, nil + return objConfig.Raw(), nil } func (cas *containerAppService) DeployYaml( @@ -322,7 +304,7 @@ func (cas *containerAppService) AddRevision( } var containers []map[string]any - if has, err = revision.GetSection(pathTemplateContainers, &containers); !has || err != nil { + if ok, err := revision.GetSection(pathTemplateContainers, &containers); !ok || err != nil { return fmt.Errorf("getting containers: %w", err) } @@ -332,8 +314,8 @@ func (cas *containerAppService) AddRevision( } // Update the container app with the new revision - var revisionTemplate map[string]any - if has, err := revision.GetSection(pathTemplate, &revisionTemplate); !has || err != nil { + revisionTemplate, ok := revision.GetMap(pathTemplate) + if !ok { return fmt.Errorf("getting revision template: %w", err) } @@ -352,15 +334,15 @@ func (cas *containerAppService) AddRevision( return fmt.Errorf("updating container app revision: %w", err) } - revisionMode, has := containerApp.GetString(pathConfigurationActiveRevisionsMode) - if !has { + revisionMode, ok := containerApp.GetString(pathConfigurationActiveRevisionsMode) + if !ok { return fmt.Errorf("getting active revisions mode: %w", err) } // If the container app is in multiple revision mode, update the traffic to point to the new revision if revisionMode == string(armappcontainers.ActiveRevisionsModeMultiple) { - revisionSuffix, has := revision.GetString(pathTemplateRevisionSuffix) - if !has { + revisionSuffix, ok := revision.GetString(pathTemplateRevisionSuffix) + if !ok { return fmt.Errorf("getting revision suffix: %w", err) } newRevisionName := fmt.Sprintf("%s--%s", appName, revisionSuffix) @@ -382,12 +364,8 @@ func (cas *containerAppService) syncSecrets( containerApp config.Config, ) (config.Config, error) { // If the container app doesn't have any existingSecrets, we don't need to do anything - var existingSecrets []any - if has, err := containerApp.GetSection(pathConfigurationSecrets, &existingSecrets); !has || err != nil { - return containerApp, err - } - - if len(existingSecrets) == 0 { + existingSecrets, ok := containerApp.GetSlice(pathConfigurationSecrets) + if !ok || len(existingSecrets) == 0 { return containerApp, nil } diff --git a/cli/azd/pkg/containerapps/container_app_test.go b/cli/azd/pkg/containerapps/container_app_test.go index 3ac1af9951b..662951b4ae5 100644 --- a/cli/azd/pkg/containerapps/container_app_test.go +++ b/cli/azd/pkg/containerapps/container_app_test.go @@ -160,3 +160,92 @@ func Test_ContainerApp_AddRevision(t *testing.T) { require.Equal(t, updatedImageName, *updatedContainerApp.Properties.Template.Containers[0].Image) require.Equal(t, "azd-0", *updatedContainerApp.Properties.Template.RevisionSuffix) } + +func Test_ContainerApp_DeployYaml(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + subscriptionId := "SUBSCRIPTION_ID" + location := "eastus2" + resourceGroup := "RESOURCE_GROUP" + appName := "APP_NAME" + + containerAppYaml := ` +location: eastus2 +name: APP_NAME +properties: + latestRevisionName: LATEST_REVISION_NAME + configuration: + activeRevisionsMode: Single + template: + containers: + - image: IMAGE_NAME +` + + expected := &armappcontainers.ContainerApp{ + Location: to.Ptr(location), + Name: to.Ptr(appName), + Properties: &armappcontainers.ContainerAppProperties{ + LatestRevisionName: to.Ptr("LATEST_REVISION_NAME"), + Configuration: &armappcontainers.Configuration{ + ActiveRevisionsMode: to.Ptr(armappcontainers.ActiveRevisionsModeSingle), + Ingress: &armappcontainers.Ingress{ + CustomDomains: []*armappcontainers.CustomDomain{ + { + Name: to.Ptr("DOMAIN_NAME"), + }, + }, + StickySessions: &armappcontainers.IngressStickySessions{ + Affinity: to.Ptr(armappcontainers.AffinitySticky), + }, + }, + }, + Template: &armappcontainers.Template{ + Containers: []*armappcontainers.Container{ + { + Image: to.Ptr("IMAGE_NAME"), + }, + }, + }, + }, + } + + containerAppGetRequest := mockazsdk.MockContainerAppGet( + mockContext, + subscriptionId, + resourceGroup, + appName, + expected, + ) + require.NotNil(t, containerAppGetRequest) + + containerAppUpdateRequest := mockazsdk.MockContainerAppCreateOrUpdate( + mockContext, + subscriptionId, + resourceGroup, + appName, + expected, + ) + require.NotNil(t, containerAppUpdateRequest) + + cas := NewContainerAppService( + mockContext.SubscriptionCredentialProvider, + clock.NewMock(), + mockContext.ArmClientOptions, + mockContext.AlphaFeaturesManager, + ) + + err := mockContext.Config.Set("alpha.aca.persistDomains", "on") + require.NoError(t, err) + err = mockContext.Config.Set("alpha.aca.persistIngressSessionAffinity", "on") + require.NoError(t, err) + + err = cas.DeployYaml(*mockContext.Context, subscriptionId, resourceGroup, appName, []byte(containerAppYaml), nil) + require.NoError(t, err) + + var actual *armappcontainers.ContainerApp + err = mocks.ReadHttpBody(containerAppUpdateRequest.Body, &actual) + require.NoError(t, err) + + require.Equal(t, expected.Properties.Configuration, actual.Properties.Configuration) + require.Equal(t, expected.Properties.Template, actual.Properties.Template) +} diff --git a/cli/azd/test/mocks/mockazsdk/container_apps.go b/cli/azd/test/mocks/mockazsdk/container_apps.go index 1d9e6d641c5..e61597ce302 100644 --- a/cli/azd/test/mocks/mockazsdk/container_apps.go +++ b/cli/azd/test/mocks/mockazsdk/container_apps.go @@ -41,6 +41,36 @@ func MockContainerAppGet( return mockRequest } +func MockContainerAppCreateOrUpdate( + mockContext *mocks.MockContext, + subscriptionId string, + resourceGroup string, + appName string, + containerApp *armappcontainers.ContainerApp, +) *http.Request { + mockRequest := &http.Request{} + + mockContext.HttpClient.When(func(request *http.Request) bool { + return request.Method == http.MethodPut && strings.Contains( + request.URL.Path, + fmt.Sprintf( + "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.App/containerApps/%s", + subscriptionId, + resourceGroup, + appName, + ), + ) + }).RespondFn(func(request *http.Request) (*http.Response, error) { + *mockRequest = *request + + response := armappcontainers.ContainerAppsClientCreateOrUpdateResponse{} + + return mocks.CreateHttpResponseWithBody(request, http.StatusCreated, response) + }) + + return mockRequest +} + func MockContainerAppUpdate( mockContext *mocks.MockContext, subscriptionId string, diff --git a/cli/azd/test/mocks/util.go b/cli/azd/test/mocks/util.go index 8b41c35f177..cbf7b54d7ae 100644 --- a/cli/azd/test/mocks/util.go +++ b/cli/azd/test/mocks/util.go @@ -31,3 +31,15 @@ func CreateEmptyHttpResponse(request *http.Request, statusCode int) (*http.Respo Body: http.NoBody, }, nil } + +// ReadHttpBody reads the body of an HTTP request or response and converts it into the specified object +func ReadHttpBody(body io.ReadCloser, v any) error { + defer body.Close() + + jsonBytes, err := io.ReadAll(body) + if err != nil { + return err + } + + return json.Unmarshal(jsonBytes, v) +}