From 4de8413a26628621b2c2a50acf904e565a3e24c3 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 28 Mar 2024 03:05:38 +0000 Subject: [PATCH 1/4] remove the code for picking services --- cli/azd/internal/repository/app_init.go | 16 ---- .../pkg/apphost/service_ingress_configurer.go | 51 ------------ cli/azd/pkg/project/dotnet_importer.go | 79 +------------------ 3 files changed, 3 insertions(+), 143 deletions(-) delete mode 100644 cli/azd/pkg/apphost/service_ingress_configurer.go diff --git a/cli/azd/internal/repository/app_init.go b/cli/azd/internal/repository/app_init.go index dc0508f1b5a..52abc174962 100644 --- a/cli/azd/internal/repository/app_init.go +++ b/cli/azd/internal/repository/app_init.go @@ -196,15 +196,6 @@ func (i *Initializer) InitFromApp( return err } - // Figure out what services to expose. - ingressSelector := apphost.NewIngressSelector(appHostManifests[appHost.Path], i.console) - tracing.SetUsageAttributes(fields.AppInitLastStep.String("modify")) - - exposed, err := ingressSelector.SelectPublicServices(ctx) - if err != nil { - return err - } - tracing.SetUsageAttributes(fields.AppInitLastStep.String("config")) // Prompt for environment before proceeding with generation @@ -212,13 +203,6 @@ func (i *Initializer) InitFromApp( if err != nil { return err } - - // Persist the configuration of the exposed services, as the user picked above. We know that the name - // of the generated import (in azure.yaml) is "app" by construction, since we are creating the user's azure.yaml - // during init. - if err := newEnv.Config.Set("services.app.config.exposedServices", exposed); err != nil { - return err - } envManager, err := i.lazyEnvManager.GetValue() if err != nil { return err diff --git a/cli/azd/pkg/apphost/service_ingress_configurer.go b/cli/azd/pkg/apphost/service_ingress_configurer.go deleted file mode 100644 index 2e39a610508..00000000000 --- a/cli/azd/pkg/apphost/service_ingress_configurer.go +++ /dev/null @@ -1,51 +0,0 @@ -package apphost - -import ( - "context" - "slices" - - "github.com/azure/azure-dev/cli/azd/pkg/input" -) - -type IngressSelector struct { - manifest *Manifest - console input.Console -} - -func NewIngressSelector(manifest *Manifest, console input.Console) *IngressSelector { - return &IngressSelector{ - manifest: manifest, - console: console, - } -} - -func (adc *IngressSelector) SelectPublicServices(ctx context.Context) ([]string, error) { - var services []string - for name, res := range adc.manifest.Resources { - // "container.v0" not supported on aspire-prev4 - // see: https://github.com/Azure/azure-dev/issues/3441 - if (res.Type == "project.v0" || res.Type == "dockerfile.v0") && len(res.Bindings) > 0 { - services = append(services, name) - } - } - - if len(services) == 0 { - return nil, nil - } - - slices.Sort(services) - - adc.console.Message(ctx, "By default, a service can only be reached from inside the Azure Container Apps environment "+ - "it is running in. Selecting a service here will also allow it to be reached from the Internet.") - - exposed, err := adc.console.MultiSelect(ctx, input.ConsoleOptions{ - Message: "Select which services to expose to the Internet", - Options: services, - DefaultValue: []string{}, - }) - if err != nil { - return nil, err - } - - return exposed, nil -} diff --git a/cli/azd/pkg/project/dotnet_importer.go b/cli/azd/pkg/project/dotnet_importer.go index 3f89ac04fbe..9c3c9bf02a4 100644 --- a/cli/azd/pkg/project/dotnet_importer.go +++ b/cli/azd/pkg/project/dotnet_importer.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io/fs" - "log" "os" "path/filepath" "strings" @@ -96,7 +95,7 @@ func (ai *DotNetImporter) CanImport(ctx context.Context, projectPath string) (bo } func (ai *DotNetImporter) ProjectInfrastructure(ctx context.Context, svcConfig *ServiceConfig) (*Infra, error) { - manifest, err := ai.ReadManifestEnsureExposedServices(ctx, svcConfig) + manifest, err := ai.ReadManifest(ctx, svcConfig) if err != nil { return nil, fmt.Errorf("generating app host manifest: %w", err) } @@ -168,7 +167,7 @@ func (ai *DotNetImporter) Services( ) (map[string]*ServiceConfig, error) { services := make(map[string]*ServiceConfig) - manifest, err := ai.ReadManifestEnsureExposedServices(ctx, svcConfig) + manifest, err := ai.ReadManifest(ctx, svcConfig) if err != nil { return nil, fmt.Errorf("generating app host manifest: %w", err) } @@ -247,7 +246,7 @@ func (ai *DotNetImporter) Services( func (ai *DotNetImporter) SynthAllInfrastructure( ctx context.Context, p *ProjectConfig, svcConfig *ServiceConfig, ) (fs.FS, error) { - manifest, err := ai.ReadManifestEnsureExposedServices(ctx, svcConfig) + manifest, err := ai.ReadManifest(ctx, svcConfig) if err != nil { return nil, fmt.Errorf("generating apphost manifest: %w", err) } @@ -365,75 +364,3 @@ func (ai *DotNetImporter) ReadManifest(ctx context.Context, svcConfig *ServiceCo ai.cache[cacheKey] = manifest return manifest, nil } - -// ReadManifestEnsureExposedServices calls ReadManifest. It also reads the value of -// the `services..config.exposedServices` property from the environment and sets the `External` property on -// each binding for the exposed services. If this key does not exist in the config for the environment, the user -// is prompted to select which services should be exposed. This can happen after an environment is created with -// `azd env new`. -func (ai *DotNetImporter) ReadManifestEnsureExposedServices( - ctx context.Context, - svcConfig *ServiceConfig) (*apphost.Manifest, error) { - manifest, err := ai.ReadManifest(ctx, svcConfig) - if err != nil { - return nil, err - } - - env, err := ai.lazyEnv.GetValue() - if err == nil { - if cfgValue, has := env.Config.Get(fmt.Sprintf("services.%s.config.exposedServices", svcConfig.Name)); has { - if exposedServices, is := cfgValue.([]interface{}); !is { - log.Printf("services.%s.config.exposedServices is not an array, ignoring setting.", svcConfig.Name) - } else { - for idx, name := range exposedServices { - if strName, ok := name.(string); !ok { - log.Printf("services.%s.config.exposedServices[%d] is not a string, ignoring value.", - svcConfig.Name, idx) - } else { - // This can happen if the user has removed a service from their app host that they previously - // had and had exposed (or changed the service such that it no longer has any bindings). - if binding, has := manifest.Resources[strName]; !has || binding.Bindings == nil { - log.Printf("service %s does not exist or has no bindings, ignoring value.", strName) - continue - } - - for _, binding := range manifest.Resources[strName].Bindings { - binding.External = true - } - } - } - } - } else { - selector := apphost.NewIngressSelector(manifest, ai.console) - exposed, err := selector.SelectPublicServices(ctx) - if err != nil { - return nil, fmt.Errorf("selecting public services: %w", err) - } - - for _, name := range exposed { - for _, binding := range manifest.Resources[name].Bindings { - binding.External = true - } - } - - err = env.Config.Set(fmt.Sprintf("services.%s.config.exposedServices", svcConfig.Name), exposed) - if err != nil { - return nil, err - } - - envManager, err := ai.lazyEnvManager.GetValue() - if err != nil { - return nil, err - } - - if err := envManager.Save(ctx, env); err != nil { - return nil, err - } - - } - } else { - log.Printf("unexpected error fetching environment: %s, exposed services may not be correct", err) - } - - return manifest, nil -} From 9320b868531df814f661af30d74707b4213bdcc6 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Thu, 28 Mar 2024 17:22:51 +0000 Subject: [PATCH 2/4] remove the logic from vs-server --- cli/azd/internal/vsrpc/environment_service_create.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cli/azd/internal/vsrpc/environment_service_create.go b/cli/azd/internal/vsrpc/environment_service_create.go index df1b890a1e0..7ae8e2f5524 100644 --- a/cli/azd/internal/vsrpc/environment_service_create.go +++ b/cli/azd/internal/vsrpc/environment_service_create.go @@ -113,18 +113,6 @@ func (s *environmentService) CreateEnvironmentAsync( azdEnv.DotenvSet(key, value) } - var servicesToExpose = make([]string, 0) - - for _, svc := range newEnv.Services { - if svc.IsExternal { - servicesToExpose = append(servicesToExpose, svc.Name) - } - } - - if err := azdEnv.Config.Set("services.app.config.exposedServices", servicesToExpose); err != nil { - return false, fmt.Errorf("setting exposed services: %w", err) - } - if err := c.envManager.Save(ctx, azdEnv); err != nil { return false, fmt.Errorf("saving new environment: %w", err) } From fb0209c7b3ab6445cd30dfd7b985b1c7fa1070b0 Mon Sep 17 00:00:00 2001 From: Wei Lim Date: Thu, 28 Mar 2024 10:54:50 -0700 Subject: [PATCH 3/4] Remove loading of exposedServices from VS --- .../vsrpc/environment_service_load.go | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/cli/azd/internal/vsrpc/environment_service_load.go b/cli/azd/internal/vsrpc/environment_service_load.go index 89695aae368..f098b183413 100644 --- a/cli/azd/internal/vsrpc/environment_service_load.go +++ b/cli/azd/internal/vsrpc/environment_service_load.go @@ -6,7 +6,6 @@ package vsrpc import ( "context" "fmt" - "slices" "strings" "github.com/azure/azure-dev/cli/azd/pkg/environment" @@ -130,26 +129,5 @@ func (s *environmentService) loadEnvironmentAsync( ret.Services = servicesFromManifest(manifest) - var exposedServices []string - - // TODO(azure/azure-dev#3284): We need to use the service name of the apphost from azure.yaml instead of assuming - // it will always be "app". "app" is just the default we use when creating an azure.yaml for the user. - val, has := e.Config.Get("services.app.config.exposedServices") - if has { - if v, ok := val.([]any); ok { - for _, svc := range v { - if s, ok := svc.(string); ok { - exposedServices = append(exposedServices, s) - } - } - } - } - - for _, svc := range ret.Services { - if slices.Contains(exposedServices, svc.Name) { - svc.IsExternal = true - } - } - return ret, nil } From dc9ad3408dd674346bd0d81629ae28eb34f62f43 Mon Sep 17 00:00:00 2001 From: Wei Lim Date: Thu, 28 Mar 2024 10:55:08 -0700 Subject: [PATCH 4/4] Remove from test files --- cli/azd/pkg/environment/environment_test.go | 9 --------- cli/azd/pkg/project/importer_test.go | 8 ++------ 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/cli/azd/pkg/environment/environment_test.go b/cli/azd/pkg/environment/environment_test.go index 9b8cfb60ad8..1a7d57d607a 100644 --- a/cli/azd/pkg/environment/environment_test.go +++ b/cli/azd/pkg/environment/environment_test.go @@ -202,15 +202,6 @@ const configSample = `{ "parameters": { "bro": "xms" } - }, - "services": { - "app": { - "config": { - "exposedServices": [ - "webapp" - ] - } - } } }` diff --git a/cli/azd/pkg/project/importer_test.go b/cli/azd/pkg/project/importer_test.go index 40a8b32a50e..7a9bf33333b 100644 --- a/cli/azd/pkg/project/importer_test.go +++ b/cli/azd/pkg/project/importer_test.go @@ -301,13 +301,9 @@ func TestImportManagerProjectInfrastructureAspire(t *testing.T) { dotnetCli: dotnet.NewDotNetCli(mockContext.CommandRunner), console: mockContext.Console, lazyEnv: lazy.NewLazy(func() (*environment.Environment, error) { - env := environment.NewWithValues("env", map[string]string{ + return environment.NewWithValues("env", map[string]string{ "DOTNET_ENVIRONMENT": "Development", - }) - // set the config to skip prompting - e := env.Config.Set("services.test.config.exposedServices", []interface{}{"test"}) - require.NoError(t, e) - return env, nil + }), nil }), lazyEnvManager: lazy.NewLazy(func() (environment.Manager, error) { return mockEnv, nil