Skip to content
Open
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
30 changes: 27 additions & 3 deletions cli/azd/cmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}",
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions cli/azd/pkg/contracts/env_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ type EnvListEnvironment struct {
IsDefault bool `json:"IsDefault"`
DotEnvPath string `json:"DotEnvPath"`
ConfigPath string `json:"ConfigPath"`
IsValid bool `json:"IsValid"`
}
1 change: 1 addition & 0 deletions cli/azd/pkg/environment/local_file_data_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
42 changes: 42 additions & 0 deletions cli/azd/pkg/environment/local_file_data_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions cli/azd/pkg/environment/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
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
Expand Down Expand Up @@ -357,6 +359,7 @@
Name: env.Name,
HasLocal: true,
DotEnvPath: env.DotEnvPath,
IsValid: env.IsValid,
}
}

Expand All @@ -372,9 +375,12 @@
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
}
Expand All @@ -400,6 +406,11 @@
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 {
Expand All @@ -414,7 +425,7 @@
if err != nil {
if m.remote == nil {
return nil, err
}

Check failure on line 428 in cli/azd/pkg/environment/manager.go

View workflow job for this annotation

GitHub Actions / azd-lint (ubuntu-latest)

printf: non-constant format string in call to fmt.Errorf (govet)

remoteEnv, err := m.remote.Get(ctx, name)
if err != nil {
Expand Down
25 changes: 25 additions & 0 deletions cli/azd/pkg/environment/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading