diff --git a/cmd/caib/buildcmd/build.go b/cmd/caib/buildcmd/build.go index be10c3dbb..87f848e8d 100644 --- a/cmd/caib/buildcmd/build.go +++ b/cmd/caib/buildcmd/build.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "time" @@ -300,6 +301,18 @@ func ApplyTargetDefaults(cmd *cobra.Command, config *buildapitypes.OperatorConfi req.ExportFormat = buildapitypes.ExportFormat(defaults.DefaultFormat) clilog.Infof("Using format %q from target defaults for %q\n", defaults.DefaultFormat, req.Target) } + + warnIfNotInList(defaults.AcceptedArchitectures, "architecture", string(req.Architecture)) + warnIfNotInList(defaults.AcceptedFormats, "format", string(req.ExportFormat)) +} + +func warnIfNotInList(accepted []string, field, value string) { + if len(accepted) == 0 || value == "" { + return + } + if !slices.Contains(accepted, value) { + _, _ = color.New(color.FgRed, color.Bold).Fprintf(os.Stderr, "Warning: %s %q is not in accepted values %v\n", field, value, accepted) + } } // displayBuildResults shows push locations after build completion. diff --git a/cmd/caib/clilog/clilog.go b/cmd/caib/clilog/clilog.go index 7d474751a..9af571405 100644 --- a/cmd/caib/clilog/clilog.go +++ b/cmd/caib/clilog/clilog.go @@ -3,7 +3,10 @@ // (stderr) and structured data output (json/yaml/table) remain visible. package clilog -import "fmt" +import ( + "fmt" + "os" +) var quiet bool @@ -26,3 +29,8 @@ func Infoln(a ...any) { fmt.Println(a...) } } + +// Warnf prints a formatted warning to stderr. Warnings are never suppressed by quiet mode. +func Warnf(format string, a ...any) { + fmt.Fprintf(os.Stderr, "Warning: "+format, a...) +} diff --git a/internal/buildapi/build_validation.go b/internal/buildapi/build_validation.go index 00f7684df..7d04de7e7 100644 --- a/internal/buildapi/build_validation.go +++ b/internal/buildapi/build_validation.go @@ -2,8 +2,10 @@ package buildapi import ( "context" + "errors" "fmt" "regexp" + "slices" "strings" "time" @@ -147,3 +149,26 @@ func validateRestoreSourcesRef(req *BuildRequest) error { req.RestoreSourcesRef = ref return nil } + +// validateTargetDefaults checks that each target's default values are within its own accepted values. +func validateTargetDefaults(targets map[string]TargetDefaults) error { + if len(targets) == 0 { + return nil + } + + var errs []string + for name, td := range targets { + checkInList(&errs, name, "architecture", td.Architecture, "acceptedArchitectures", td.AcceptedArchitectures) + checkInList(&errs, name, "defaultFormat", td.DefaultFormat, "acceptedFormats", td.AcceptedFormats) + } + if len(errs) > 0 { + return errors.New(strings.Join(errs, "; ")) + } + return nil +} + +func checkInList(errs *[]string, target, field, value, listName string, accepted []string) { + if value != "" && len(accepted) > 0 && !slices.Contains(accepted, value) { + *errs = append(*errs, fmt.Sprintf("target %q: %s %q not in %s %v", target, field, value, listName, accepted)) + } +} diff --git a/internal/buildapi/build_validation_test.go b/internal/buildapi/build_validation_test.go index 179b2e804..54c1afcf2 100644 --- a/internal/buildapi/build_validation_test.go +++ b/internal/buildapi/build_validation_test.go @@ -194,3 +194,100 @@ var _ = Describe("validateRestoreSourcesRef", func() { Expect(req.RestoreSourcesRef).ToNot(HavePrefix(" ")) }) }) + +var _ = Describe("validateTargetDefaults", func() { + It("passes when targets map is nil", func() { + Expect(validateTargetDefaults(nil)).To(Succeed()) + }) + + It("passes when no accepted lists are set", func() { + targets := map[string]TargetDefaults{ + "qemu": {Architecture: "arm64", DefaultFormat: "raw"}, + } + Expect(validateTargetDefaults(targets)).To(Succeed()) + }) + + It("passes when defaults match accepted values", func() { + targets := map[string]TargetDefaults{ + "qemu": { + DefaultFormat: "raw", + AcceptedFormats: []string{"qcow2", "raw"}, + AcceptedArchitectures: []string{"amd64", "arm64"}, + }, + "ebbr": { + Architecture: "arm64", + DefaultFormat: "simg", + AcceptedFormats: []string{"simg"}, + AcceptedArchitectures: []string{"arm64"}, + }, + } + Expect(validateTargetDefaults(targets)).To(Succeed()) + }) + + It("rejects architecture not in target's accepted list", func() { + targets := map[string]TargetDefaults{ + "bad-board": { + Architecture: "mips64", + AcceptedArchitectures: []string{"amd64", "arm64"}, + }, + } + err := validateTargetDefaults(targets) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("bad-board")) + Expect(err.Error()).To(ContainSubstring("mips64")) + Expect(err.Error()).To(ContainSubstring("acceptedArchitectures")) + }) + + It("rejects defaultFormat not in target's accepted list", func() { + targets := map[string]TargetDefaults{ + "my-target": { + DefaultFormat: "vdi", + AcceptedFormats: []string{"qcow2", "raw", "simg"}, + }, + } + err := validateTargetDefaults(targets) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("my-target")) + Expect(err.Error()).To(ContainSubstring("vdi")) + }) + + It("skips validation when default field is empty", func() { + targets := map[string]TargetDefaults{ + "qemu": { + DefaultFormat: "raw", + AcceptedFormats: []string{"qcow2", "raw"}, + AcceptedArchitectures: []string{"amd64", "arm64"}, + }, + } + Expect(validateTargetDefaults(targets)).To(Succeed()) + }) + + It("skips validation when accepted list is empty", func() { + targets := map[string]TargetDefaults{ + "qemu": { + Architecture: "anything", + DefaultFormat: "whatever", + AcceptedFormats: []string{}, + AcceptedArchitectures: []string{}, + }, + } + Expect(validateTargetDefaults(targets)).To(Succeed()) + }) + + It("reports errors from multiple targets", func() { + targets := map[string]TargetDefaults{ + "a": { + Architecture: "bad-arch", + AcceptedArchitectures: []string{"amd64", "arm64"}, + }, + "b": { + DefaultFormat: "bad-fmt", + AcceptedFormats: []string{"qcow2", "raw", "simg"}, + }, + } + err := validateTargetDefaults(targets) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("bad-arch")) + Expect(err.Error()).To(ContainSubstring("bad-fmt")) + }) +}) diff --git a/internal/buildapi/server.go b/internal/buildapi/server.go index ea15ca210..d94702a60 100644 --- a/internal/buildapi/server.go +++ b/internal/buildapi/server.go @@ -166,9 +166,11 @@ var loadTargetDefaultsFn = func( var parsed struct { Targets map[string]struct { - Architecture string `yaml:"architecture"` - ExtraArgs []string `yaml:"extraArgs"` - DefaultFormat string `yaml:"defaultFormat"` + Architecture string `yaml:"architecture"` + ExtraArgs []string `yaml:"extraArgs"` + DefaultFormat string `yaml:"defaultFormat"` + AcceptedFormats []string `yaml:"acceptedFormats"` + AcceptedArchitectures []string `yaml:"acceptedArchitectures"` } `yaml:"targets"` } if err := yaml.Unmarshal([]byte(data), &parsed); err != nil { @@ -178,11 +180,18 @@ var loadTargetDefaultsFn = func( result := make(map[string]TargetDefaults, len(parsed.Targets)) for name, t := range parsed.Targets { result[name] = TargetDefaults{ - Architecture: t.Architecture, - ExtraArgs: t.ExtraArgs, - DefaultFormat: t.DefaultFormat, + Architecture: t.Architecture, + ExtraArgs: t.ExtraArgs, + DefaultFormat: t.DefaultFormat, + AcceptedFormats: t.AcceptedFormats, + AcceptedArchitectures: t.AcceptedArchitectures, } } + + if err := validateTargetDefaults(result); err != nil { + return nil, fmt.Errorf("invalid target-defaults.yaml: %w", err) + } + return result, nil } diff --git a/internal/buildapi/server_test.go b/internal/buildapi/server_test.go index 40f90dca7..2286ed865 100644 --- a/internal/buildapi/server_test.go +++ b/internal/buildapi/server_test.go @@ -622,6 +622,43 @@ var _ = Describe("APIServer", func() { ExtraArgs: []string{"--separate-partitions"}, })) }) + + It("should return per-target validation hints in target defaults", func() { + config := &automotivev1alpha1.OperatorConfig{ + Spec: automotivev1alpha1.OperatorConfigSpec{}, + } + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return nil, nil + } + loadOperatorConfigFn = func(_ context.Context, _ ctrlclient.Client, _ string) (*automotivev1alpha1.OperatorConfig, error) { + return config, nil + } + loadTargetDefaultsFn = func(_ context.Context, _ ctrlclient.Client, _ string) (map[string]TargetDefaults, error) { + return map[string]TargetDefaults{ + "qemu": { + DefaultFormat: "raw", + AcceptedFormats: []string{"qcow2", "raw"}, + AcceptedArchitectures: []string{"amd64", "arm64"}, + }, + }, nil + } + + req, err := http.NewRequest(http.MethodGet, "/v1/config", nil) + Expect(err).NotTo(HaveOccurred()) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = req + c.Set("reqID", "test-req-id") + + server.handleGetOperatorConfig(c) + + Expect(w.Code).To(Equal(http.StatusOK)) + var response OperatorConfigResponse + Expect(json.Unmarshal(w.Body.Bytes(), &response)).To(Succeed()) + Expect(response.TargetDefaults).To(HaveLen(1)) + Expect(response.TargetDefaults["qemu"].AcceptedFormats).To(ConsistOf("qcow2", "raw")) + Expect(response.TargetDefaults["qemu"].AcceptedArchitectures).To(ConsistOf("amd64", "arm64")) + }) }) Context("Server Lifecycle", func() { diff --git a/internal/buildapi/types.go b/internal/buildapi/types.go index 55076a8a6..fb99c8ee2 100644 --- a/internal/buildapi/types.go +++ b/internal/buildapi/types.go @@ -340,11 +340,13 @@ type JumpstarterTarget struct { FlashCmd string `json:"flashCmd,omitempty"` } -// TargetDefaults contains build defaults for a target (from ConfigMap) +// TargetDefaults contains build defaults and validation hints for a target (from ConfigMap) type TargetDefaults struct { - Architecture string `json:"architecture,omitempty"` - ExtraArgs []string `json:"extraArgs,omitempty"` - DefaultFormat string `json:"defaultFormat,omitempty"` + Architecture string `json:"architecture,omitempty"` + ExtraArgs []string `json:"extraArgs,omitempty"` + DefaultFormat string `json:"defaultFormat,omitempty"` + AcceptedFormats []string `json:"acceptedFormats,omitempty"` + AcceptedArchitectures []string `json:"acceptedArchitectures,omitempty"` } // OperatorConfigResponse returns relevant operator configuration for CLI validation diff --git a/internal/controller/operatorconfig/controller.go b/internal/controller/operatorconfig/controller.go index 9bb82dbc3..be0f63116 100644 --- a/internal/controller/operatorconfig/controller.go +++ b/internal/controller/operatorconfig/controller.go @@ -53,55 +53,81 @@ var targetDefaultsYAML = `targets: extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ridesx4_r3: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ridesx4_scmi: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ride4_sa8775p_sx_r3: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ride4_sa8775p_sx: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ride4_sa8775p_sx_legacy: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ride4_sa8775p_sx_legacy_r3: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ride4_sa8650p_sx_r3: architecture: arm64 extraArgs: ["--separate-partitions"] include: ["system_a", "system_b", "boot_a", "boot_b"] defaultFormat: "simg" + acceptedFormats: ["simg"] + acceptedArchitectures: ["arm64"] ebbr: architecture: arm64 defaultFormat: "simg" + acceptedFormats: ["simg", "raw"] + acceptedArchitectures: ["arm64"] rcar_s4: architecture: arm64 defaultFormat: "simg" + acceptedFormats: ["simg", "raw"] + acceptedArchitectures: ["arm64"] j784s4evm: architecture: arm64 defaultFormat: "simg" + acceptedFormats: ["simg", "raw"] + acceptedArchitectures: ["arm64"] s32g_vnp_rdb3: architecture: arm64 defaultFormat: "simg" + acceptedFormats: ["simg", "raw"] + acceptedArchitectures: ["arm64"] qemu: defaultFormat: "raw" + acceptedFormats: ["qcow2", "raw"] + acceptedArchitectures: ["amd64", "arm64", "x86_64", "aarch64"] ` // isNoMatchError checks if error is "no matches for kind" error (CRD doesn't exist)