From 58488b398ed886591e825913fb319787c8465701 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 17 Jan 2024 14:39:28 +0100 Subject: [PATCH] validate merge result vs individual overrides Signed-off-by: Nicolas De Loof --- loader/extends.go | 10 +++++-- loader/loader.go | 16 ++++++------ loader/override_test.go | 37 ++++++++++++++++++++++++++ types/healthcheck.go | 2 +- types/project.go | 2 +- types/types.go | 58 ++++++++++++++++++++--------------------- validation/external.go | 5 ++++ 7 files changed, 89 insertions(+), 41 deletions(-) diff --git a/loader/extends.go b/loader/extends.go index a5614f43..a56ca1f1 100644 --- a/loader/extends.go +++ b/loader/extends.go @@ -31,9 +31,15 @@ func ApplyExtends(ctx context.Context, dict map[string]any, workingdir string, o if !ok { return nil } - services := a.(map[string]any) + services, ok := a.(map[string]any) + if !ok { + return fmt.Errorf("services must be a mapping") + } for name, s := range services { - service := s.(map[string]any) + service, ok := s.(map[string]any) + if !ok { + return fmt.Errorf("services.%s must be a mapping", name) + } x, ok := service["extends"] if !ok { continue diff --git a/loader/loader.go b/loader/loader.go index 58ca98aa..05c51bac 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -312,12 +312,6 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option fixEmptyNotNull(cfg) - if !opts.SkipValidation { - if err := schema.Validate(cfg); err != nil { - return fmt.Errorf("validating %s: %w", file.Filename, err) - } - } - if !opts.SkipExtends { err = ApplyExtends(fctx, cfg, config.WorkingDir, opts, ct, processors...) if err != nil { @@ -333,6 +327,12 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option dict, err = override.Merge(dict, cfg) + if !opts.SkipValidation { + if err := schema.Validate(dict); err != nil { + return fmt.Errorf("validating %s: %w", file.Filename, err) + } + } + return err } @@ -378,8 +378,6 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option } } - dict = groupXFieldsIntoExtensions(dict, tree.NewPath()) - if !opts.SkipValidation { if err := validation.Validate(dict); err != nil { return nil, err @@ -423,6 +421,8 @@ func load(ctx context.Context, configDetails types.ConfigDetails, opts *Options, Environment: configDetails.Environment, } delete(dict, "name") // project name set by yaml must be identified by caller as opts.projectName + + dict = groupXFieldsIntoExtensions(dict, tree.NewPath()) err = Transform(dict, project) if err != nil { return nil, err diff --git a/loader/override_test.go b/loader/override_test.go index 25758b1f..34e5f075 100644 --- a/loader/override_test.go +++ b/loader/override_test.go @@ -128,3 +128,40 @@ services: assert.NilError(t, err) assert.Check(t, p.Services["test"].DependsOn["foo"].Required == false) } + +func TestOverridePartial(t *testing.T) { + yaml := ` +name: test-override-networks +services: + test: + image: test + depends_on: + foo: + condition: service_healthy + + foo: + image: foo +` + + override := ` +services: + test: + depends_on: + foo: + # This is invalid according to json schema as condition is required + required: false +` + _, err := LoadWithContext(context.Background(), types.ConfigDetails{ + ConfigFiles: []types.ConfigFile{ + { + Filename: "base", + Content: []byte(yaml), + }, + { + Filename: "override", + Content: []byte(override), + }, + }, + }) + assert.NilError(t, err) +} diff --git a/types/healthcheck.go b/types/healthcheck.go index 1bbf5e9e..c6c3b37e 100644 --- a/types/healthcheck.go +++ b/types/healthcheck.go @@ -30,7 +30,7 @@ type HealthCheckConfig struct { StartInterval *Duration `yaml:"start_interval,omitempty" json:"start_interval,omitempty"` Disable bool `yaml:"disable,omitempty" json:"disable,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // HealthCheckTest is the command run to test the health of a service diff --git a/types/project.go b/types/project.go index a50fd9b3..e4cee7e7 100644 --- a/types/project.go +++ b/types/project.go @@ -45,7 +45,7 @@ type Project struct { Volumes Volumes `yaml:"volumes,omitempty" json:"volumes,omitempty"` Secrets Secrets `yaml:"secrets,omitempty" json:"secrets,omitempty"` Configs Configs `yaml:"configs,omitempty" json:"configs,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` // https://github.com/golang/go/issues/6213 + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` // https://github.com/golang/go/issues/6213 // IncludeReferences is keyed by Compose YAML filename and contains config for // other Compose YAML files it directly triggered a load of via `include`. diff --git a/types/types.go b/types/types.go index 103db084..72cfc817 100644 --- a/types/types.go +++ b/types/types.go @@ -132,7 +132,7 @@ type ServiceConfig struct { VolumesFrom []string `yaml:"volumes_from,omitempty" json:"volumes_from,omitempty"` WorkingDir string `yaml:"working_dir,omitempty" json:"working_dir,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // MarshalYAML makes ServiceConfig implement yaml.Marshaller @@ -280,7 +280,7 @@ type BuildConfig struct { Platforms StringList `yaml:"platforms,omitempty" json:"platforms,omitempty"` Privileged bool `yaml:"privileged,omitempty" json:"privileged,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // BlkioConfig define blkio config @@ -292,7 +292,7 @@ type BlkioConfig struct { DeviceWriteBps []ThrottleDevice `yaml:"device_write_bps,omitempty" json:"device_write_bps,omitempty"` DeviceWriteIOps []ThrottleDevice `yaml:"device_write_iops,omitempty" json:"device_write_iops,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // WeightDevice is a structure that holds device:weight pair @@ -300,7 +300,7 @@ type WeightDevice struct { Path string Weight uint16 - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ThrottleDevice is a structure that holds device:rate_per_second pair @@ -308,7 +308,7 @@ type ThrottleDevice struct { Path string Rate UnitBytes - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // MappingWithColon is a mapping type that can be converted from a list of @@ -320,7 +320,7 @@ type LoggingConfig struct { Driver string `yaml:"driver,omitempty" json:"driver,omitempty"` Options Options `yaml:"options,omitempty" json:"options,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // DeployConfig the deployment configuration for a service @@ -335,7 +335,7 @@ type DeployConfig struct { Placement Placement `yaml:"placement,omitempty" json:"placement,omitempty"` EndpointMode string `yaml:"endpoint_mode,omitempty" json:"endpoint_mode,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // UpdateConfig the service update configuration @@ -347,7 +347,7 @@ type UpdateConfig struct { MaxFailureRatio float32 `yaml:"max_failure_ratio,omitempty" json:"max_failure_ratio,omitempty"` Order string `yaml:"order,omitempty" json:"order,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // Resources the resource limits and reservations @@ -355,7 +355,7 @@ type Resources struct { Limits *Resource `yaml:"limits,omitempty" json:"limits,omitempty"` Reservations *Resource `yaml:"reservations,omitempty" json:"reservations,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // Resource is a resource to be limited or reserved @@ -367,7 +367,7 @@ type Resource struct { Devices []DeviceRequest `yaml:"devices,omitempty" json:"devices,omitempty"` GenericResources []GenericResource `yaml:"generic_resources,omitempty" json:"generic_resources,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // GenericResource represents a "user defined" resource which can @@ -375,7 +375,7 @@ type Resource struct { type GenericResource struct { DiscreteResourceSpec *DiscreteGenericResource `yaml:"discrete_resource_spec,omitempty" json:"discrete_resource_spec,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // DiscreteGenericResource represents a "user defined" resource which is defined @@ -386,7 +386,7 @@ type DiscreteGenericResource struct { Kind string `json:"kind"` Value int64 `json:"value"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // RestartPolicy the service restart policy @@ -396,7 +396,7 @@ type RestartPolicy struct { MaxAttempts *uint64 `yaml:"max_attempts,omitempty" json:"max_attempts,omitempty"` Window *Duration `yaml:"window,omitempty" json:"window,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // Placement constraints for the service @@ -405,14 +405,14 @@ type Placement struct { Preferences []PlacementPreferences `yaml:"preferences,omitempty" json:"preferences,omitempty"` MaxReplicas uint64 `yaml:"max_replicas_per_node,omitempty" json:"max_replicas_per_node,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // PlacementPreferences is the preferences for a service placement type PlacementPreferences struct { Spread string `yaml:"spread,omitempty" json:"spread,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ServiceNetworkConfig is the network configuration for a service @@ -424,7 +424,7 @@ type ServiceNetworkConfig struct { LinkLocalIPs []string `yaml:"link_local_ips,omitempty" json:"link_local_ips,omitempty"` MacAddress string `yaml:"mac_address,omitempty" json:"mac_address,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ServicePortConfig is the port configuration for a service @@ -435,7 +435,7 @@ type ServicePortConfig struct { Published string `yaml:"published,omitempty" json:"published,omitempty"` Protocol string `yaml:"protocol,omitempty" json:"protocol,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ParsePortConfig parse short syntax for service port configuration @@ -488,7 +488,7 @@ type ServiceVolumeConfig struct { Volume *ServiceVolumeVolume `yaml:"volume,omitempty" json:"volume,omitempty"` Tmpfs *ServiceVolumeTmpfs `yaml:"tmpfs,omitempty" json:"tmpfs,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // String render ServiceVolumeConfig as a volume string, one can parse back using loader.ParseVolume @@ -534,7 +534,7 @@ type ServiceVolumeBind struct { Propagation string `yaml:"propagation,omitempty" json:"propagation,omitempty"` CreateHostPath bool `yaml:"create_host_path,omitempty" json:"create_host_path,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // SELinux represents the SELinux re-labeling options. @@ -565,7 +565,7 @@ const ( type ServiceVolumeVolume struct { NoCopy bool `yaml:"nocopy,omitempty" json:"nocopy,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ServiceVolumeTmpfs are options for a service volume of type tmpfs @@ -574,7 +574,7 @@ type ServiceVolumeTmpfs struct { Mode uint32 `yaml:"mode,omitempty" json:"mode,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // FileReferenceConfig for a reference to a swarm file object @@ -585,7 +585,7 @@ type FileReferenceConfig struct { GID string `yaml:"gid,omitempty" json:"gid,omitempty"` Mode *uint32 `yaml:"mode,omitempty" json:"mode,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // ServiceConfigObjConfig is the config obj configuration for a service @@ -600,7 +600,7 @@ type UlimitsConfig struct { Soft int `yaml:"soft,omitempty" json:"soft,omitempty"` Hard int `yaml:"hard,omitempty" json:"hard,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // MarshalYAML makes UlimitsConfig implement yaml.Marshaller @@ -637,14 +637,14 @@ type NetworkConfig struct { Attachable bool `yaml:"attachable,omitempty" json:"attachable,omitempty"` Labels Labels `yaml:"labels,omitempty" json:"labels,omitempty"` EnableIPv6 bool `yaml:"enable_ipv6,omitempty" json:"enable_ipv6,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // IPAMConfig for a network type IPAMConfig struct { Driver string `yaml:"driver,omitempty" json:"driver,omitempty"` Config []*IPAMPool `yaml:"config,omitempty" json:"config,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // IPAMPool for a network @@ -663,7 +663,7 @@ type VolumeConfig struct { DriverOpts Options `yaml:"driver_opts,omitempty" json:"driver_opts,omitempty"` External External `yaml:"external,omitempty" json:"external,omitempty"` Labels Labels `yaml:"labels,omitempty" json:"labels,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // External identifies a Volume or Network as a reference to a resource that is @@ -675,7 +675,7 @@ type CredentialSpecConfig struct { Config string `yaml:"config,omitempty" json:"config,omitempty"` // Config was added in API v1.40 File string `yaml:"file,omitempty" json:"file,omitempty"` Registry string `yaml:"registry,omitempty" json:"registry,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } // FileObjectConfig is a config type for a file used by a service @@ -689,7 +689,7 @@ type FileObjectConfig struct { Driver string `yaml:"driver,omitempty" json:"driver,omitempty"` DriverOpts map[string]string `yaml:"driver_opts,omitempty" json:"driver_opts,omitempty"` TemplateDriver string `yaml:"template_driver,omitempty" json:"template_driver,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` } const ( @@ -708,7 +708,7 @@ type DependsOnConfig map[string]ServiceDependency type ServiceDependency struct { Condition string `yaml:"condition,omitempty" json:"condition,omitempty"` Restart bool `yaml:"restart,omitempty" json:"restart,omitempty"` - Extensions Extensions `yaml:"#extensions,inline" json:"-"` + Extensions Extensions `yaml:"#extensions,inline,omitempty" json:"-"` Required bool `yaml:"required" json:"required"` } diff --git a/validation/external.go b/validation/external.go index c9c1a3c9..b74d551a 100644 --- a/validation/external.go +++ b/validation/external.go @@ -18,6 +18,7 @@ package validation import ( "fmt" + "strings" "github.com/compose-spec/compose-go/v2/consts" "github.com/compose-spec/compose-go/v2/tree" @@ -37,6 +38,10 @@ func checkExternal(v map[string]any, p tree.Path) error { case "name", "external", consts.Extensions: continue default: + if strings.HasPrefix(k, "x-") { + // custom extension, ignored + continue + } return fmt.Errorf("%s: conflicting parameters \"external\" and %q specified", p, k) } }