From 16932c16a48318ac693ee85af540976193370b8c Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Tue, 21 Jul 2026 16:35:07 +0000 Subject: [PATCH] Make required variable reporting deterministic Signed-off-by: Ousama Ben Younes --- cmd/compose/config.go | 3 +- pkg/compose/loader.go | 42 +++++++++++++++++++++ pkg/compose/loader_test.go | 37 +++++++++++++++++++ pkg/compose/variables.go | 69 +++++++++++++++++++++++++++++++++++ pkg/compose/variables_test.go | 40 ++++++++++++++++++++ 5 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 pkg/compose/variables.go create mode 100644 pkg/compose/variables_test.go diff --git a/cmd/compose/config.go b/cmd/compose/config.go index 8dfa847ea33..6960f595d75 100644 --- a/cmd/compose/config.go +++ b/cmd/compose/config.go @@ -29,7 +29,6 @@ import ( "github.com/compose-spec/compose-go/v2/cli" "github.com/compose-spec/compose-go/v2/loader" - "github.com/compose-spec/compose-go/v2/template" "github.com/compose-spec/compose-go/v2/types" "github.com/docker/cli/cli/command" "github.com/sirupsen/logrus" @@ -532,7 +531,7 @@ func runVariables(ctx context.Context, dockerCli command.Cli, opts configOptions return err } - variables := template.ExtractVariables(model, template.DefaultPattern) + variables := compose.ExtractVariables(model) if opts.Format == "yaml" { result, err := yaml.Marshal(variables) diff --git a/pkg/compose/loader.go b/pkg/compose/loader.go index 9a0699da7c6..67098b042f2 100644 --- a/pkg/compose/loader.go +++ b/pkg/compose/loader.go @@ -19,7 +19,9 @@ package compose import ( "context" "errors" + "fmt" "os" + "sort" "strings" "github.com/compose-spec/compose-go/v2/cli" @@ -53,6 +55,10 @@ func (s *composeService) LoadProject(ctx context.Context, options api.ProjectLoa api.Separator = "_" } + if err := s.validateRequiredVariables(ctx, options, remoteLoaders); err != nil { + return nil, err + } + project, err := projectOptions.LoadProject(ctx) if err != nil { return nil, err @@ -67,6 +73,42 @@ func (s *composeService) LoadProject(ctx context.Context, options api.ProjectLoa return project, nil } +func (s *composeService) validateRequiredVariables(ctx context.Context, options api.ProjectLoadOptions, remoteLoaders []loader.ResourceLoader) error { + precheckOptions := options + precheckOptions.ProjectOptionsFns = append([]cli.ProjectOptionsFn{}, options.ProjectOptionsFns...) + precheckOptions.ProjectOptionsFns = append(precheckOptions.ProjectOptionsFns, + cli.WithInterpolation(false), + cli.WithLoadOptions(loader.WithSkipValidation), + cli.WithoutEnvironmentResolution, + ) + + projectOptions, err := s.buildProjectOptions(precheckOptions, remoteLoaders) + if err != nil { + return err + } + + model, err := projectOptions.LoadModel(ctx) + if err != nil { + return err + } + + var missing []string + variables := ExtractVariables(model) + for name, variable := range variables { + value, ok := projectOptions.Environment.Resolve(name) + if variable.Required && (!ok || value == "") { + missing = append(missing, name) + } + } + + if len(missing) == 0 { + return nil + } + + sort.Strings(missing) + return fmt.Errorf("required variable %s is missing a value", missing[0]) +} + // createRemoteLoaders creates Git and OCI remote loaders if not in offline mode func (s *composeService) createRemoteLoaders(options api.ProjectLoadOptions) []loader.ResourceLoader { if options.Offline { diff --git a/pkg/compose/loader_test.go b/pkg/compose/loader_test.go index 979db0aa91f..62cad7a00f9 100644 --- a/pkg/compose/loader_test.go +++ b/pkg/compose/loader_test.go @@ -28,6 +28,25 @@ import ( "github.com/docker/compose/v5/pkg/api" ) +const ( + requiredVariablesProjectName = "required-vars" + requiredVariablesExpectedErr = "required variable TIMEZONE is missing a value" + requiredVariablesComposeYAML = ` +services: + traefik: + image: traefik:v3.6.11 + environment: + - TZ=${TIMEZONE:?} + volumes: + - type: bind + source: ${TRAEFIK_ACME_PATH:?} + target: /acme/ + - type: bind + source: ${TRAEFIK_LOGS_PATH:?} + target: /logs +` +) + func TestLoadProject_Basic(t *testing.T) { // Create a temporary compose file tmpDir := t.TempDir() @@ -68,6 +87,24 @@ services: assert.Equal(t, "web", webService.CustomLabels[api.ServiceLabel]) } +func TestLoadProjectReportsRequiredVariablesDeterministically(t *testing.T) { + tmpDir := t.TempDir() + composeFile := filepath.Join(tmpDir, "compose.yaml") + err := os.WriteFile(composeFile, []byte(requiredVariablesComposeYAML), 0o644) + assert.NilError(t, err) + + service, err := NewComposeService(nil) + assert.NilError(t, err) + + for range 100 { + _, err = service.LoadProject(t.Context(), api.ProjectLoadOptions{ + ProjectName: requiredVariablesProjectName, + ConfigPaths: []string{composeFile}, + }) + assert.ErrorContains(t, err, requiredVariablesExpectedErr) + } +} + func TestLoadProject_WithEnvironmentResolution(t *testing.T) { tmpDir := t.TempDir() composeFile := filepath.Join(tmpDir, "compose.yaml") diff --git a/pkg/compose/variables.go b/pkg/compose/variables.go new file mode 100644 index 00000000000..33e6ac3562e --- /dev/null +++ b/pkg/compose/variables.go @@ -0,0 +1,69 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "sort" + + "github.com/compose-spec/compose-go/v2/template" +) + +const variableExtractionValueKey = "value" + +func ExtractVariables(model map[string]any) map[string]template.Variable { + variables := map[string]template.Variable{} + extractVariables(model, variables) + return variables +} + +func extractVariables(value any, variables map[string]template.Variable) { + switch value := value.(type) { + case string: + mergeVariables(variables, template.ExtractVariables(map[string]any{variableExtractionValueKey: value}, template.DefaultPattern)) + case map[string]any: + keys := make([]string, 0, len(value)) + for key := range value { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + extractVariables(value[key], variables) + } + case []any: + for _, elem := range value { + extractVariables(elem, variables) + } + } +} + +func mergeVariables(dst map[string]template.Variable, src map[string]template.Variable) { + for name, variable := range src { + current, ok := dst[name] + if !ok { + dst[name] = variable + continue + } + current.Required = current.Required || variable.Required + if current.DefaultValue == "" { + current.DefaultValue = variable.DefaultValue + } + if current.PresenceValue == "" { + current.PresenceValue = variable.PresenceValue + } + dst[name] = current + } +} diff --git a/pkg/compose/variables_test.go b/pkg/compose/variables_test.go new file mode 100644 index 00000000000..f967a56de12 --- /dev/null +++ b/pkg/compose/variables_test.go @@ -0,0 +1,40 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +const variableMergeDomainName = "PIHOLE_DOMAIN" + +func TestExtractVariablesKeepsRequiredOccurrence(t *testing.T) { + variables := ExtractVariables(map[string]any{ + "services": map[string]any{ + "pihole": map[string]any{ + "labels": []any{ + "traefik.http.routers.pihole.rule=Host(`${PIHOLE_DOMAIN:?}`)", + "traefik.http.middlewares.pihole-redirect.redirectregex.regex=^(https://${PIHOLE_DOMAIN})/?$", + }, + }, + }, + }) + + assert.Assert(t, variables[variableMergeDomainName].Required) +}