Skip to content

feat(cli): add --id-label flag to read-configuration command #244

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.

Sign up for GitHub

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

Merged
skevetter merged 1 commit into from
May 6, 2026
Merged

feat(cli): add --id-label flag to read-configuration command #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 54 additions & 2 deletions cmd/readconfiguration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type ReadConfigurationCmd struct {
WorkspaceFolder string
Config string
ContainerID string
IDLabels []string
IncludeFeaturesConfiguration bool
IncludeMergedConfiguration bool
}
Expand Down Expand Up @@ -55,6 +56,13 @@ func NewReadConfigurationCmd(f *flags.GlobalFlags) *cobra.Command {
"",
"Path to a specific devcontainer.json",
)
readConfigCmd.Flags().
StringArrayVar(
&cmd.IDLabels,
"id-label",
nil,
"Override the default container identification labels (format: key=value, can be specified multiple times)",
)
readConfigCmd.Flags().
BoolVar(
&cmd.IncludeFeaturesConfiguration,
Expand Down Expand Up @@ -89,6 +97,10 @@ func (cmd *ReadConfigurationCmd) Run(
c *cobra.Command,
_ []string,
) error {
if err := config2.ValidateIDLabels(cmd.IDLabels); err != nil {
return err
}

parsedConfig, workspaceFolder, err := cmd.resolve(c.Context())
if err != nil {
return err
Expand Down Expand Up @@ -129,14 +141,17 @@ func (cmd *ReadConfigurationCmd) resolve(ctx context.Context) (
string,
error,
) {
if cmd.ContainerID == "" && cmd.WorkspaceFolder == "" {
if cmd.ContainerID == "" && cmd.WorkspaceFolder == "" && len(cmd.IDLabels) == 0 {
return nil, "", fmt.Errorf(
"either --workspace-folder or --container-id must be provided",
"either --workspace-folder, --container-id, or --id-label must be provided",
)
}
if cmd.ContainerID != "" {
return cmd.resolveConfigFromContainer(ctx)
}
if len(cmd.IDLabels) > 0 {
return cmd.resolveConfigFromIDLabels(ctx)
}
Comment on lines +144 to +154

@coderabbitai coderabbitai Bot May 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject conflicting selectors instead of silently preferring one.

If a caller passes more than one of --workspace-folder, --container-id, or --id-label, this branch just ignores the lower-priority inputs. That can return configuration from the wrong source, and it is extra confusing because Run still validates --id-label even when resolve later ignores it. Please fail fast unless exactly one selector is set.

Proposed fix 
 func (cmd *ReadConfigurationCmd) resolve(ctx context.Context) (
 	*config2.DevContainerConfig,
 	string,
 	error,
 ) {
-	if cmd.ContainerID == "" && cmd.WorkspaceFolder == "" && len(cmd.IDLabels) == 0 {
+	provided := 0
+	if cmd.ContainerID != "" {
+		provided++
+	}
+	if cmd.WorkspaceFolder != "" {
+		provided++
+	}
+	if len(cmd.IDLabels) > 0 {
+		provided++
+	}
+	if provided == 0 {
 		return nil, "", fmt.Errorf(
 			"either --workspace-folder, --container-id, or --id-label must be provided",
 		)
 	}
+	if provided > 1 {
+		return nil, "", fmt.Errorf(
+			"only one of --workspace-folder, --container-id, or --id-label may be provided",
+		)
+	}
 	if cmd.ContainerID != "" {
 		return cmd.resolveConfigFromContainer(ctx)
 	}
 	if len(cmd.IDLabels) > 0 {
 		return cmd.resolveConfigFromIDLabels(ctx)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/readconfiguration.go` around lines 144 - 154, The current selector logic
silently prefers one input when multiple selectors are provided; change it to
validate that exactly one of cmd.WorkspaceFolder, cmd.ContainerID, or
cmd.IDLabels is set and return a clear error if more than one is provided.
Update the branch that currently chooses between cmd.resolveConfigFromContainer
and cmd.resolveConfigFromIDLabels to first compute how many selectors are
non-empty (checking cmd.WorkspaceFolder != "", cmd.ContainerID != "", and
len(cmd.IDLabels) > 0), fail fast with fmt.Errorf when count != 1, and only then
call the appropriate resolver (e.g., resolveConfigFromWorkspace /
resolveConfigFromContainer / resolveConfigFromIDLabels) based on which single
selector is set. Ensure error text names the conflicting flags so callers can
correct their input.

return cmd.resolveConfig()
}

Expand Down Expand Up @@ -229,6 +244,43 @@ func (cmd *ReadConfigurationCmd) resolveConfigFromContainer(ctx context.Context)
return parsedConfig, workspaceFolder, nil
}

func (cmd *ReadConfigurationCmd) resolveConfigFromIDLabels(ctx context.Context) (
*config2.DevContainerConfig,
string,
error,
) {
containerDetails, err := findRunningContainer(
ctx, defaultDockerCommand, "", cmd.IDLabels,
)
if err != nil {
return nil, "", err
}

subCtx := &config2.SubstitutionContext{}
imageMetadata, err := metadata.GetImageMetadataFromContainer(
containerDetails,
subCtx,
)
if err != nil {
return nil, "", fmt.Errorf("get image metadata from container: %w", err)
}

parsedConfig := &config2.DevContainerConfig{}
if len(imageMetadata.Config) > 0 {
last := imageMetadata.Config[len(imageMetadata.Config)-1]
parsedConfig.DevContainerConfigBase = last.DevContainerConfigBase
parsedConfig.DevContainerActions = last.DevContainerActions
parsedConfig.NonComposeBase = last.NonComposeBase
}

workspaceFolder := containerDetails.Config.WorkingDir
if workspaceFolder == "" {
workspaceFolder = "/"
}

return parsedConfig, workspaceFolder, nil
}

func buildMergedConfig(
parsedConfig *config2.DevContainerConfig,
) (*config2.MergedDevContainerConfig, error) {
Expand Down
35 changes: 35 additions & 0 deletions e2e/tests/readconfiguration/readconfiguration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,41 @@ var _ = ginkgo.Describe("read-configuration command", ginkgo.Label("read-configu
gomega.Expect(hr["storage"]).To(gomega.Equal("32gb"))
}, ginkgo.SpecTimeout(framework.TimeoutShort()))

ginkgo.It("should read configuration using custom id-label",
func(ctx context.Context) {
tempDir, err := framework.CopyToTempDirWithoutChdir(
"tests/readconfiguration/testdata",
)
framework.ExpectNoError(err)
ginkgo.DeferCleanup(func() { _ = os.RemoveAll(tempDir) })

f, err := framework.SetupDockerProvider(initialDir+"/bin", "docker")
framework.ExpectNoError(err)
ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, tempDir)

err = f.DevsyUp(ctx, tempDir,
"--id-label", "devsy.readconfig.test=custom")
framework.ExpectNoError(err)

stdout, _, err := f.ExecCommandCapture(ctx, []string{
"read-configuration",
"--id-label", "devsy.readconfig.test=custom",
"--include-merged-configuration",
})
framework.ExpectNoError(err)

var result map[string]any
err = json.Unmarshal([]byte(stdout), &result)
framework.ExpectNoError(err, "output should be valid JSON")

gomega.Expect(result).To(gomega.HaveKey("configuration"))
gomega.Expect(result).To(gomega.HaveKey("mergedConfiguration"))

ws, ok := result["workspace"].(map[string]any)
gomega.Expect(ok).To(gomega.BeTrue(), "workspace should be an object")
gomega.Expect(ws).To(gomega.HaveKey("workspaceFolder"))
}, ginkgo.SpecTimeout(framework.TimeoutShort()))

ginkgo.It("reads configuration from a running container via --container-id",
func(ctx context.Context) {
f := framework.NewDefaultFramework(initialDir + "/bin")
Expand Down
Loading