diff --git a/cli/azd/cmd/env.go b/cli/azd/cmd/env.go index d4bd9ff2612..96b50f6f51d 100644 --- a/cli/azd/cmd/env.go +++ b/cli/azd/cmd/env.go @@ -821,6 +821,10 @@ func (e *envListAction) Run(ctx context.Context) (*actions.ActionResult, error) Heading: "DEFAULT", ValueTemplate: "{{.IsDefault}}", }, + { + Heading: "STATUS", + ValueTemplate: "{{if .IsValid}}✓{{else}}⚠ INVALID NAME{{end}}", + }, { Heading: "LOCAL", ValueTemplate: "{{.HasLocal}}", @@ -834,11 +838,31 @@ func (e *envListAction) Run(ctx context.Context) (*actions.ActionResult, error) err = e.formatter.Format(envs, e.writer, output.TableFormatterOptions{ Columns: columns, }) + if err != nil { + return nil, err + } + + // Count invalid environments and display warning if any exist + invalidCount := 0 + for _, env := range envs { + if !env.IsValid { + invalidCount++ + } + } + + if invalidCount > 0 { + warning := fmt.Sprintf( + "\n⚠ Warning: %d environment(s) have invalid names and may cause issues with azd commands. "+ + "Environment names must follow the same naming rules enforced when creating or renaming environments.\n", + invalidCount, + ) + fmt.Fprint(e.writer, warning) + } } else { err = e.formatter.Format(envs, e.writer, nil) - } - if err != nil { - return nil, err + if err != nil { + return nil, err + } } return nil, nil diff --git a/cli/azd/pkg/contracts/env_list.go b/cli/azd/pkg/contracts/env_list.go index 0b597ca185c..cc51b014a69 100644 --- a/cli/azd/pkg/contracts/env_list.go +++ b/cli/azd/pkg/contracts/env_list.go @@ -8,4 +8,5 @@ type EnvListEnvironment struct { IsDefault bool `json:"IsDefault"` DotEnvPath string `json:"DotEnvPath"` ConfigPath string `json:"ConfigPath"` + IsValid bool `json:"IsValid"` } diff --git a/cli/azd/pkg/environment/local_file_data_store.go b/cli/azd/pkg/environment/local_file_data_store.go index dab3339b56e..ee5d1bde121 100644 --- a/cli/azd/pkg/environment/local_file_data_store.go +++ b/cli/azd/pkg/environment/local_file_data_store.go @@ -70,6 +70,7 @@ func (fs *LocalFileDataStore) List(ctx context.Context) ([]*contracts.EnvListEnv IsDefault: ent.Name() == defaultEnv, DotEnvPath: filepath.Join(fs.azdContext.EnvironmentRoot(ent.Name()), DotEnvFileName), ConfigPath: filepath.Join(fs.azdContext.EnvironmentRoot(ent.Name()), ConfigFileName), + IsValid: IsValidEnvironmentName(ent.Name()), } envs = append(envs, ev) } diff --git a/cli/azd/pkg/environment/local_file_data_store_test.go b/cli/azd/pkg/environment/local_file_data_store_test.go index 39754f7b17a..59b9ace29d2 100644 --- a/cli/azd/pkg/environment/local_file_data_store_test.go +++ b/cli/azd/pkg/environment/local_file_data_store_test.go @@ -5,10 +5,12 @@ package environment import ( "context" + "os" "path/filepath" "testing" "github.com/azure/azure-dev/cli/azd/pkg/config" + "github.com/azure/azure-dev/cli/azd/pkg/contracts" "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/test/mocks" "github.com/stretchr/testify/require" @@ -40,6 +42,46 @@ func Test_LocalFileDataStore_List(t *testing.T) { require.NoError(t, err) require.NotNil(t, envList) }) + + t.Run("ValidatesEnvironmentNames", func(t *testing.T) { + // Create a fresh temporary directory for this test to avoid interference + tempDir := t.TempDir() + freshAzdContext := azdcontext.NewAzdContextWithDirectory(tempDir) + freshDataStore := NewLocalFileDataStore(freshAzdContext, fileConfigManager) + + // Create an environment with a valid name + validEnv := New("valid-env") + err := freshDataStore.Save(*mockContext.Context, validEnv, nil) + require.NoError(t, err) + + // Manually create a directory with an invalid name + invalidEnvDir := filepath.Join(freshAzdContext.EnvironmentDirectory(), "#invalid$name") + err = os.MkdirAll(invalidEnvDir, 0755) + require.NoError(t, err) + + // List environments + envList, err := freshDataStore.List(*mockContext.Context) + require.NoError(t, err) + require.NotNil(t, envList) + require.Equal(t, 2, len(envList)) + + // Check that valid environment has IsValid=true + var validEnvResult *contracts.EnvListEnvironment + var invalidEnvResult *contracts.EnvListEnvironment + for _, env := range envList { + if env.Name == "valid-env" { + validEnvResult = env + } else if env.Name == "#invalid$name" { + invalidEnvResult = env + } + } + + require.NotNil(t, validEnvResult, "valid-env should be in the list") + require.True(t, validEnvResult.IsValid, "valid-env should be marked as valid") + + require.NotNil(t, invalidEnvResult, "#invalid$name should be in the list") + require.False(t, invalidEnvResult.IsValid, "#invalid$name should be marked as invalid") + }) } func Test_LocalFileDataStore_SaveAndGet(t *testing.T) { diff --git a/cli/azd/pkg/environment/manager.go b/cli/azd/pkg/environment/manager.go index 6a38e1c58ad..6dddc5ba56a 100644 --- a/cli/azd/pkg/environment/manager.go +++ b/cli/azd/pkg/environment/manager.go @@ -31,6 +31,8 @@ type Description struct { HasRemote bool // Specifies when the environment is the default environment IsDefault bool + // Specifies whether the environment name is valid + IsValid bool } // Spec is the specification for creating a new environment @@ -357,6 +359,7 @@ func (m *manager) List(ctx context.Context) ([]*Description, error) { Name: env.Name, HasLocal: true, DotEnvPath: env.DotEnvPath, + IsValid: env.IsValid, } } @@ -372,9 +375,12 @@ func (m *manager) List(ctx context.Context) ([]*Description, error) { existing = &Description{ Name: env.Name, HasRemote: true, + IsValid: env.IsValid, } } else { existing.HasRemote = true + // For environments that exist both locally and remotely, + // use the local validity check (already set above) } envMap[env.Name] = existing } @@ -400,6 +406,11 @@ func (m *manager) Get(ctx context.Context, name string) (*Environment, error) { return nil, ErrNameNotSpecified } + // Validate environment name + if !IsValidEnvironmentName(name) { + return nil, fmt.Errorf(invalidEnvironmentNameMsg(name)) + } + // Check cache first cached, err := m.getFromCache(ctx, name) if err != nil { diff --git a/cli/azd/pkg/environment/manager_test.go b/cli/azd/pkg/environment/manager_test.go index 1d965545812..d9ac1fb6335 100644 --- a/cli/azd/pkg/environment/manager_test.go +++ b/cli/azd/pkg/environment/manager_test.go @@ -281,6 +281,31 @@ func Test_EnvManager_Get(t *testing.T) { localDataStore.AssertCalled(t, "Save", *mockContext.Context, foundEnv, mock.Anything) }) + + t.Run("InvalidEnvironmentName", func(t *testing.T) { + localDataStore := &MockDataStore{} + + manager := newManagerForTest(azdContext, mockContext.Console, localDataStore, nil) + + // Test various invalid names + invalidNames := []string{ + "#invalid", + "$bad%name", + "no spaces", + "no*asterisk", + } + + for _, invalidName := range invalidNames { + env, err := manager.Get(*mockContext.Context, invalidName) + require.Error(t, err, "Should error for invalid name: %s", invalidName) + require.Nil(t, env) + require.Contains(t, err.Error(), "invalid") + require.Contains(t, err.Error(), "alphanumeric") + } + + // localDataStore.Get should never be called for invalid names + localDataStore.AssertNotCalled(t, "Get") + }) } func Test_EnvManager_Save(t *testing.T) {