From ddfa58492696079347829455445ed3a156b057da Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 18 Apr 2024 11:31:39 -0700 Subject: [PATCH 1/3] Adds ability to override resource group at the azd service level --- cli/azd/internal/cmd/util.go | 7 ++++++- cli/azd/pkg/osutil/expandable_string.go | 5 +++++ cli/azd/pkg/project/resource_manager.go | 18 ++++++++++++++---- cli/azd/pkg/project/service_config.go | 2 ++ cli/azd/pkg/project/service_manager.go | 13 ++++++++++++- cli/azd/pkg/project/service_target_aks_test.go | 4 ++-- schemas/alpha/azure.yaml.json | 5 +++++ 7 files changed, 46 insertions(+), 8 deletions(-) diff --git a/cli/azd/internal/cmd/util.go b/cli/azd/internal/cmd/util.go index 36a3a7d2a1a..a65337ebbe1 100644 --- a/cli/azd/internal/cmd/util.go +++ b/cli/azd/internal/cmd/util.go @@ -27,7 +27,12 @@ func getResourceGroupFollowUp( } subscriptionId := env.GetSubscriptionId() - if resourceGroupName, err := resourceManager.GetResourceGroupName(ctx, subscriptionId, projectConfig); err == nil { + resourceGroupName, err := resourceManager.GetResourceGroupName( + ctx, + subscriptionId, + projectConfig.ResourceGroupName, + ) + if err == nil { defaultFollowUpText := fmt.Sprintf( "You can view the resources created under the resource group %s in Azure Portal:", resourceGroupName) if whatIf { diff --git a/cli/azd/pkg/osutil/expandable_string.go b/cli/azd/pkg/osutil/expandable_string.go index ee5495f9294..fb0714c9679 100644 --- a/cli/azd/pkg/osutil/expandable_string.go +++ b/cli/azd/pkg/osutil/expandable_string.go @@ -20,6 +20,11 @@ type ExpandableString struct { template string } +// Empty returns true if the template is empty. +func (e ExpandableString) Empty() bool { + return e.template == "" +} + // Envsubst evaluates the template, substituting values as [envsubst.Eval] would. func (e ExpandableString) Envsubst(mapping func(string) string) (string, error) { return envsubst.Eval(e.template, mapping) diff --git a/cli/azd/pkg/project/resource_manager.go b/cli/azd/pkg/project/resource_manager.go index d28eef8f874..55e5cc957be 100644 --- a/cli/azd/pkg/project/resource_manager.go +++ b/cli/azd/pkg/project/resource_manager.go @@ -11,6 +11,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azureutil" "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/infra" + "github.com/azure/azure-dev/cli/azd/pkg/osutil" "github.com/azure/azure-dev/cli/azd/pkg/tools/azcli" ) @@ -18,7 +19,11 @@ import ( // This would typically be used during deployment when azd need to deploy applications // to the Azure resource hosting the application type ResourceManager interface { - GetResourceGroupName(ctx context.Context, subscriptionId string, projectConfig *ProjectConfig) (string, error) + GetResourceGroupName( + ctx context.Context, + subscriptionId string, + resourceGroupTemplate osutil.ExpandableString, + ) (string, error) GetServiceResources( ctx context.Context, subscriptionId string, @@ -69,9 +74,9 @@ func NewResourceManager( func (rm *resourceManager) GetResourceGroupName( ctx context.Context, subscriptionId string, - projectConfig *ProjectConfig, + resourceGroupTemplate osutil.ExpandableString, ) (string, error) { - name, err := projectConfig.ResourceGroupName.Envsubst(rm.env.Getenv) + name, err := resourceGroupTemplate.Envsubst(rm.env.Getenv) if err != nil { return "", err } @@ -197,7 +202,12 @@ func (rm *resourceManager) GetTargetResource( subscriptionId string, serviceConfig *ServiceConfig, ) (*environment.TargetResource, error) { - resourceGroupName, err := rm.GetResourceGroupName(ctx, subscriptionId, serviceConfig.Project) + resourceGroupTemplate := serviceConfig.ResourceGroupName + if resourceGroupTemplate.Empty() { + resourceGroupTemplate = serviceConfig.Project.ResourceGroupName + } + + resourceGroupName, err := rm.GetResourceGroupName(ctx, subscriptionId, resourceGroupTemplate) if err != nil { return nil, err } diff --git a/cli/azd/pkg/project/service_config.go b/cli/azd/pkg/project/service_config.go index 22e96e70ac3..2f1d8641c6f 100644 --- a/cli/azd/pkg/project/service_config.go +++ b/cli/azd/pkg/project/service_config.go @@ -14,6 +14,8 @@ type ServiceConfig struct { Project *ProjectConfig `yaml:"-"` // The friendly name/key of the project from the azure.yaml file Name string `yaml:"-"` + // The azure resource group to deploy the service to + ResourceGroupName osutil.ExpandableString `yaml:"resourceGroup,omitempty"` // The name used to override the default azure resource name ResourceName osutil.ExpandableString `yaml:"resourceName,omitempty"` // The relative path to the project folder from the project root diff --git a/cli/azd/pkg/project/service_manager.go b/cli/azd/pkg/project/service_manager.go index 4ed8e74aca7..26feec6fb49 100644 --- a/cli/azd/pkg/project/service_manager.go +++ b/cli/azd/pkg/project/service_manager.go @@ -474,8 +474,19 @@ func (sm *serviceManager) Deploy( containerEnvName = parts[len(parts)-1] } + // Get any explicitly configured resource group name + // 1. Service level override + // 2. Project level override + resourceGroupNameTemplate := serviceConfig.ResourceGroupName + if resourceGroupNameTemplate.Empty() { + resourceGroupNameTemplate = serviceConfig.Project.ResourceGroupName + } + resourceGroupName, err := sm.resourceManager.GetResourceGroupName( - ctx, sm.env.GetSubscriptionId(), serviceConfig.Project) + ctx, + sm.env.GetSubscriptionId(), + resourceGroupNameTemplate, + ) if err != nil { task.SetError(fmt.Errorf("getting resource group name: %w", err)) return diff --git a/cli/azd/pkg/project/service_target_aks_test.go b/cli/azd/pkg/project/service_target_aks_test.go index 84b39bf832b..a13532aac5e 100644 --- a/cli/azd/pkg/project/service_target_aks_test.go +++ b/cli/azd/pkg/project/service_target_aks_test.go @@ -925,9 +925,9 @@ type MockResourceManager struct { func (m *MockResourceManager) GetResourceGroupName( ctx context.Context, subscriptionId string, - projectConfig *ProjectConfig, + resourceGroupTemplate osutil.ExpandableString, ) (string, error) { - args := m.Called(ctx, subscriptionId, projectConfig) + args := m.Called(ctx, subscriptionId, resourceGroupTemplate) return args.String(0), args.Error(1) } diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index a389b0315d2..2fc2ad8510b 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -69,6 +69,11 @@ "host" ], "properties": { + "resourceGroup": { + "type": "string", + "title": "Name of the Azure resource group that contains the resource", + "description": "By default, the CLI will discover the Azure resource within texpandable_string_testhe default resource group. When specified, the CLI will instead find the Azure resource within the matching resource group. Supports environment variable substitution." + }, "resourceName": { "type": "string", "title": "Name of the Azure resource that implements the service", From c695f1ed3f651542e878482f2e38ee88b7613899 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 18 Apr 2024 11:42:22 -0700 Subject: [PATCH 2/3] Fixed typo in schema --- schemas/alpha/azure.yaml.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index 2fc2ad8510b..22b82f83d51 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -72,7 +72,7 @@ "resourceGroup": { "type": "string", "title": "Name of the Azure resource group that contains the resource", - "description": "By default, the CLI will discover the Azure resource within texpandable_string_testhe default resource group. When specified, the CLI will instead find the Azure resource within the matching resource group. Supports environment variable substitution." + "description": "By default, the CLI will discover the Azure resource within the default resource group. When specified, the CLI will instead find the Azure resource within the specified resource group. Supports environment variable substitution." }, "resourceName": { "type": "string", From f0f7a4ad6deb28b6209ec0b832cadc1b8fe9ed42 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Fri, 19 Apr 2024 12:16:07 -0700 Subject: [PATCH 3/3] Adds unit tests for resource manager --- cli/azd/pkg/osutil/expandable_string_test.go | 12 ++ cli/azd/pkg/project/resource_manager_test.go | 177 +++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 cli/azd/pkg/project/resource_manager_test.go diff --git a/cli/azd/pkg/osutil/expandable_string_test.go b/cli/azd/pkg/osutil/expandable_string_test.go index 8a8db9e30b4..4e9832dbf50 100644 --- a/cli/azd/pkg/osutil/expandable_string_test.go +++ b/cli/azd/pkg/osutil/expandable_string_test.go @@ -23,3 +23,15 @@ func TestExpandableStringYaml(t *testing.T) { assert.Equal(t, "${foo}\n", string(marshalled)) } + +func TestExpandableString_Empty(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + e := NewExpandableString("") + assert.True(t, e.Empty()) + }) + + t.Run("NonEmpty", func(t *testing.T) { + e := NewExpandableString("${ENV_VAR}") + assert.False(t, e.Empty()) + }) +} diff --git a/cli/azd/pkg/project/resource_manager_test.go b/cli/azd/pkg/project/resource_manager_test.go new file mode 100644 index 00000000000..6b0851242f7 --- /dev/null +++ b/cli/azd/pkg/project/resource_manager_test.go @@ -0,0 +1,177 @@ +package project + +import ( + "context" + "fmt" + "net/http" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" + "github.com/azure/azure-dev/cli/azd/pkg/convert" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/osutil" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockazcli" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type testInitFunc func(*mocks.MockContext) + +// Validates that the resource group is correctly resolved from different configuration +// 1. Resource group referenced in service config +// 2. Resource group referenced in project +// 3. Resource group referenced in environment variable +// 4. Resource group tagged with azd-env-name +func Test_ResourceManager_GetTargetResource(t *testing.T) { + taggedResourceGroup := &armresources.ResourceGroup{ + ID: convert.RefOf(fmt.Sprintf( + "/subscriptions/%s/resourceGroups/%s", + "SUBSCRIPTION_id", + "TAGGED_RESOURCE_GROUP", + )), + Name: convert.RefOf("TAGGED_RESOURCE_GROUP"), + Type: convert.RefOf("Microsoft.Resources/resourceGroups"), + Location: convert.RefOf("eastus2"), + } + + fromProjectConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageJavaScript) + fromProjectConfig.Project.ResourceGroupName = osutil.NewExpandableString("PROJECT_RESOURCE_GROUP") + + fromServiceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageJavaScript) + fromServiceConfig.Project.ResourceGroupName = osutil.NewExpandableString("PROJECT_RESOURCE_GROUP") + fromServiceConfig.ResourceGroupName = osutil.NewExpandableString("SERVICE_RESOURCE_GROUP") + + tests := []struct { + name string + env *environment.Environment + serviceConfig *ServiceConfig + expectedResourceGroup string + init testInitFunc + }{ + { + name: "ResourceGroupFromTag", + init: func(mockContext *mocks.MockContext) { + setupGetResourceGroupMock(mockContext, taggedResourceGroup) + }, + env: environment.NewWithValues("test", map[string]string{ + environment.SubscriptionIdEnvVarName: "SUBSCRIPTION_ID", + }), + serviceConfig: createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageJavaScript), + expectedResourceGroup: "TAGGED_RESOURCE_GROUP", + }, + { + name: "ResourceGroupFromEnvVar", + env: environment.NewWithValues("test", map[string]string{ + environment.ResourceGroupEnvVarName: "ENV_VAR_RESOURCE_GROUP", + environment.SubscriptionIdEnvVarName: "SUBSCRIPTION_ID", + }), + serviceConfig: createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageJavaScript), + expectedResourceGroup: "ENV_VAR_RESOURCE_GROUP", + }, + { + name: "ResourceGroupFromProject", + env: environment.NewWithValues("test", map[string]string{ + environment.ResourceGroupEnvVarName: "ENV_VAR_RESOURCE_GROUP", + environment.SubscriptionIdEnvVarName: "SUBSCRIPTION_ID", + }), + serviceConfig: fromProjectConfig, + expectedResourceGroup: "PROJECT_RESOURCE_GROUP", + }, + { + name: "ResourceGroupFromService", + env: environment.NewWithValues("test", map[string]string{ + environment.ResourceGroupEnvVarName: "ENV_VAR_RESOURCE_GROUP", + environment.SubscriptionIdEnvVarName: "SUBSCRIPTION_ID", + }), + serviceConfig: fromServiceConfig, + expectedResourceGroup: "SERVICE_RESOURCE_GROUP", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + azCli := mockazcli.NewAzCliFromMockContext(mockContext) + mockDeploymentOperations := &mockDeploymentOperations{} + + if tt.init != nil { + tt.init(mockContext) + } + + expectedResource := &armresources.GenericResourceExpanded{ + ID: convert.RefOf("RESOURCE_ID"), + Name: convert.RefOf("RESOURCE_NAME"), + Type: convert.RefOf("Microsoft.Web/sites"), + Location: convert.RefOf("eastus2"), + } + + setupGetResourceMock(mockContext, expectedResource) + + resourceManager := NewResourceManager(tt.env, azCli, mockDeploymentOperations) + targetResource, err := resourceManager.GetTargetResource( + *mockContext.Context, + tt.env.GetSubscriptionId(), + tt.serviceConfig, + ) + + require.NoError(t, err) + require.NotNil(t, targetResource) + require.Equal(t, tt.expectedResourceGroup, targetResource.ResourceGroupName()) + require.Equal(t, "RESOURCE_NAME", targetResource.ResourceName()) + require.Equal(t, tt.env.GetSubscriptionId(), targetResource.SubscriptionId()) + }) + } +} + +type mockDeploymentOperations struct { + mock.Mock +} + +func setupGetResourceGroupMock(mockContext *mocks.MockContext, resourceGroup *armresources.ResourceGroup) { + mockContext.HttpClient.When(func(request *http.Request) bool { + return strings.HasSuffix(request.URL.Path, "/resourcegroups") && strings.Contains(request.URL.RawQuery, "filter=") + }).RespondFn(func(request *http.Request) (*http.Response, error) { + result := armresources.ResourceGroupListResult{ + Value: []*armresources.ResourceGroup{ + resourceGroup, + }, + } + + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, result) + }) +} + +func setupGetResourceMock(mockContext *mocks.MockContext, resource *armresources.GenericResourceExpanded) { + mockContext.HttpClient.When(func(request *http.Request) bool { + return strings.HasSuffix(request.URL.Path, "/resources") && strings.Contains(request.URL.RawQuery, "filter=") + }).RespondFn(func(request *http.Request) (*http.Response, error) { + result := armresources.ResourceListResult{ + Value: []*armresources.GenericResourceExpanded{ + resource, + }, + } + + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, result) + }) +} + +func (m *mockDeploymentOperations) ListSubscriptionDeploymentOperations( + ctx context.Context, + subscriptionId string, + deploymentName string, +) ([]*armresources.DeploymentOperation, error) { + args := m.Called(ctx, subscriptionId, deploymentName) + return args.Get(0).([]*armresources.DeploymentOperation), args.Error(1) +} + +func (m *mockDeploymentOperations) ListResourceGroupDeploymentOperations( + ctx context.Context, + subscriptionId string, + resourceGroupName string, + deploymentName string, +) ([]*armresources.DeploymentOperation, error) { + args := m.Called(ctx, subscriptionId, resourceGroupName, deploymentName) + return args.Get(0).([]*armresources.DeploymentOperation), args.Error(1) +}