Skip to content
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

remove the code for picking services #3611

Merged
merged 5 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions cli/azd/internal/repository/app_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,29 +196,13 @@ 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
newEnv, err := initializeEnv()
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
Expand Down
12 changes: 0 additions & 12 deletions cli/azd/internal/vsrpc/environment_service_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
22 changes: 0 additions & 22 deletions cli/azd/internal/vsrpc/environment_service_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package vsrpc
import (
"context"
"fmt"
"slices"
"strings"

"github.com/azure/azure-dev/cli/azd/pkg/environment"
Expand Down Expand Up @@ -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
}
51 changes: 0 additions & 51 deletions cli/azd/pkg/apphost/service_ingress_configurer.go

This file was deleted.

9 changes: 0 additions & 9 deletions cli/azd/pkg/environment/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,6 @@ const configSample = `{
"parameters": {
"bro": "xms"
}
},
"services": {
"app": {
"config": {
"exposedServices": [
"webapp"
]
}
}
}
}`

Expand Down
79 changes: 3 additions & 76 deletions cli/azd/pkg/project/dotnet_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"
"io/fs"
"log"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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.<name>.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
}
8 changes: 2 additions & 6 deletions cli/azd/pkg/project/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading