diff --git a/configs/adapter-config-template.yaml b/configs/adapter-config-template.yaml index 4a83e12..906c0a1 100644 --- a/configs/adapter-config-template.yaml +++ b/configs/adapter-config-template.yaml @@ -73,6 +73,14 @@ spec: # Global params # ============================================================================ # params to extract from CloudEvent and environment variables + # + # SUPPORTED TYPES: + # ================ + # - string: Default, any value converted to string + # - int/int64: Integer value (strings parsed, floats truncated) + # - float/float64: Floating point value + # - bool: Boolean (supports: true/false, yes/no, on/off, 1/0) + # params: # Environment variables from deployment - name: "hyperfleetApiBaseUrl" @@ -94,6 +102,20 @@ spec: description: "Unique identifier for the target cluster" required: true + # Example: Extract and convert to int + # - name: "nodeCount" + # source: "event.spec.nodeCount" + # type: "int" + # default: 3 + # description: "Number of nodes in the cluster" + + # Example: Extract and convert to bool + # - name: "enableFeature" + # source: "env.ENABLE_FEATURE" + # type: "bool" + # default: false + # description: "Enable experimental feature" + # ============================================================================ # Global Preconditions diff --git a/go.mod b/go.mod index dd8c07b..e6f8cea 100644 --- a/go.mod +++ b/go.mod @@ -52,12 +52,16 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.9.0 // indirect + github.com/gabriel-vasile/mimetype v1.4.12 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.23.0 // indirect + github.com/go-playground/locales v0.14.1 // indirect + github.com/go-playground/universal-translator v0.18.1 // indirect + github.com/go-playground/validator/v10 v10.30.1 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/gnostic-models v0.7.0 // indirect @@ -71,6 +75,7 @@ require ( github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/klauspost/compress v1.18.0 // indirect + github.com/leodido/go-urn v1.4.0 // indirect github.com/lithammer/shortuuid/v3 v3.0.7 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/magiconair/properties v1.8.10 // indirect @@ -121,13 +126,13 @@ require ( go.opentelemetry.io/otel/trace v1.36.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.43.0 // indirect + golang.org/x/crypto v0.46.0 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect - golang.org/x/net v0.45.0 // indirect + golang.org/x/net v0.47.0 // indirect golang.org/x/oauth2 v0.30.0 // indirect golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.37.0 // indirect - golang.org/x/term v0.36.0 // indirect + golang.org/x/sys v0.39.0 // indirect + golang.org/x/term v0.38.0 // indirect golang.org/x/time v0.12.0 // indirect google.golang.org/api v0.243.0 // indirect google.golang.org/genproto v0.0.0-20250603155806-513f23925822 // indirect diff --git a/go.sum b/go.sum index e91d12f..58b498e 100644 --- a/go.sum +++ b/go.sum @@ -91,6 +91,8 @@ github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= github.com/fxamacker/cbor/v2 v2.9.0 h1:NpKPmjDBgUfBms6tr6JZkTHtfFGcMKsw3eGcmD/sapM= github.com/fxamacker/cbor/v2 v2.9.0/go.mod h1:vM4b+DJCtHn+zz7h3FFp/hDAI9WNWCsZj23V5ytsSxQ= +github.com/gabriel-vasile/mimetype v1.4.12 h1:e9hWvmLYvtp846tLHam2o++qitpguFiYCKbn0w9jyqw= +github.com/gabriel-vasile/mimetype v1.4.12/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -108,6 +110,12 @@ github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+GrE= github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= +github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= +github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= +github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= +github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= +github.com/go-playground/validator/v10 v10.30.1 h1:f3zDSN/zOma+w6+1Wswgd9fLkdwy06ntQJp0BBvFG0w= +github.com/go-playground/validator/v10 v10.30.1/go.mod h1:oSuBIQzuJxL//3MelwSLD5hc2Tu889bF0Idm9Dg26cM= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= @@ -180,6 +188,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= +github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/lithammer/shortuuid/v3 v3.0.7 h1:trX0KTHy4Pbwo/6ia8fscyHoGA+mf1jWbPJVuvyJQQ8= github.com/lithammer/shortuuid/v3 v3.0.7/go.mod h1:vMk8ke37EmiewwolSO1NLW8vP4ZaKlRuDIi8tWWmAts= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= @@ -344,6 +354,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.43.0 h1:dduJYIi3A3KOfdGOHX8AVZ/jGiyPa3IbBozJ5kNuE04= golang.org/x/crypto v0.43.0/go.mod h1:BFbav4mRNlXJL4wNeejLpWxB7wMbc79PdRGhWKncxR0= +golang.org/x/crypto v0.46.0 h1:cKRW/pmt1pKAfetfu+RCEvjvZkA9RimPbh7bhFjGVBU= +golang.org/x/crypto v0.46.0/go.mod h1:Evb/oLKmMraqjZ2iQTwDwvCtJkczlDuTmdJXoZVzqU0= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 h1:e66Fs6Z+fZTbFBAxKfP3PALWBtpfqks2bwGcexMxgtk= golang.org/x/exp v0.0.0-20240909161429-701f63a606c0/go.mod h1:2TbTHSBQa924w8M6Xs1QcRcFwyucIwBGpK1p2f1YFFY= @@ -363,6 +375,8 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.45.0 h1:RLBg5JKixCy82FtLJpeNlVM0nrSqpCRYzVU1n8kj0tM= golang.org/x/net v0.45.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY= +golang.org/x/net v0.47.0 h1:Mx+4dIFzqraBXUugkia1OOvlD6LemFo1ALMHjrXDOhY= +golang.org/x/net v0.47.0/go.mod h1:/jNxtkgq5yWUGYkaZGqo27cfGZ1c5Nen03aYrrKpVRU= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= @@ -385,8 +399,12 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ= golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.36.0 h1:zMPR+aF8gfksFprF/Nc/rd1wRS1EI6nDBGyWAvDzx2Q= golang.org/x/term v0.36.0/go.mod h1:Qu394IJq6V6dCBRgwqshf3mPF85AqzYEzofzRdZkWss= +golang.org/x/term v0.38.0 h1:PQ5pkm/rLO6HnxFR7N2lJHOZX6Kez5Y1gDSJla6jo7Q= +golang.org/x/term v0.38.0/go.mod h1:bSEAKrOT1W+VSu9TSCMtoGEOUcKxOKgl3LE5QEF/xVg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= diff --git a/internal/config_loader/README.md b/internal/config_loader/README.md index 1eb11ce..fc70e3c 100644 --- a/internal/config_loader/README.md +++ b/internal/config_loader/README.md @@ -5,10 +5,22 @@ The `config_loader` package loads and validates HyperFleet Adapter configuration ## Features - **YAML Parsing**: Load configurations from files or bytes -- **Validation**: Required fields, structure, CEL expressions, K8s manifests -- **Type Safety**: Strongly-typed Go structs +- **Structural Validation**: Required fields, formats, enums via `go-playground/validator` +- **Semantic Validation**: CEL expressions, template variables, K8s manifests +- **Type Safety**: Strongly-typed Go structs with struct embedding - **Helper Methods**: Query params, resources, preconditions by name +## Package Structure + +| File | Purpose | +|------|---------| +| `loader.go` | Load configs from file/bytes, resolve file references | +| `types.go` | All type definitions with validation tags | +| `validator.go` | Orchestrates structural + semantic validation | +| `struct_validator.go` | `go-playground/validator` integration | +| `accessors.go` | Helper methods for querying config | +| `constants.go` | Field names, API versions, regex patterns | + ## Usage ```go @@ -63,19 +75,52 @@ See `configs/adapter-config-template.yaml` for the complete configuration refere ## Validation -The loader validates: -- Required fields (`apiVersion`, `kind`, `metadata.name`, `adapter.version`) -- HTTP methods in API calls (`GET`, `POST`, `PUT`, `PATCH`, `DELETE`) -- Parameters have `source` -- File references exist (`buildRef`, `manifest.ref`) -- CEL expressions are syntactically valid -- K8s manifests have required fields -- CaptureField has either `field` or `expression` (not both, not neither) +### Two-Phase Validation + +1. **Structural Validation** (`ValidateStructure`) + - Uses `go-playground/validator` with struct tags + - Required fields, enum values, mutual exclusivity + - Custom validators: `resourcename`, `validoperator` + +2. **Semantic Validation** (`ValidateSemantic`) + - CEL expression syntax + - Template variable references + - Condition value types + - K8s manifest required fields + +### Validation Tags + +```go +// Required field +Name string `yaml:"name" validate:"required"` + +// Enum validation +Method string `yaml:"method" validate:"required,oneof=GET POST PUT PATCH DELETE"` + +// Mutual exclusivity (field OR expression, not both) +Field string `yaml:"field,omitempty" validate:"required_without=Expression,excluded_with=Expression"` +Expression string `yaml:"expression,omitempty" validate:"required_without=Field,excluded_with=Field"` + +// Custom validators +Name string `yaml:"name" validate:"required,resourcename"` +Operator string `yaml:"operator" validate:"required,validoperator"` +``` + +### Custom Validators + +| Tag | Purpose | +|-----|---------| +| `resourcename` | CEL-compatible names (lowercase start, no hyphens) | +| `validoperator` | Valid condition operators (eq, neq, in, notIn, exists) | + +### Error Messages Validation errors are descriptive: ``` spec.params[0].name is required -spec.preconditions[1].apiCall.method must be one of: GET, POST, PUT, PATCH, DELETE +spec.preconditions[1].apiCall.method "INVALID" is invalid (allowed: GET, POST, PUT, PATCH, DELETE) +spec.resources[0].name "my-resource": must start with lowercase letter and contain only letters, numbers, underscores (no hyphens) +spec.preconditions[0].capture[0]: must have either 'field' or 'expression' set ``` ## Types @@ -89,8 +134,43 @@ spec.preconditions[1].apiCall.method must be one of: GET, POST, PUT, PATCH, DELE | `PostConfig` | Post-processing actions | | `APICall` | HTTP request configuration | | `Condition` | Field/operator/value condition | -| `CaptureField` | Field capture from API response (see below) | -| `ValueDef` | Dynamic value definition in payload builds (see below) | +| `CaptureField` | Field capture from API response | +| `ValueDef` | Dynamic value definition in payload builds | +| `ValidationErrors` | Collection of validation errors | + +### Struct Embedding + +The package uses struct embedding to reduce duplication: + +```go +// ActionBase - common fields for actions (preconditions, post-actions) +type ActionBase struct { + Name string `yaml:"name" validate:"required"` + APICall *APICall `yaml:"apiCall,omitempty"` +} + +// FieldExpressionDef - field OR expression (mutually exclusive) +type FieldExpressionDef struct { + Field string `yaml:"field,omitempty" validate:"required_without=Expression,excluded_with=Expression"` + Expression string `yaml:"expression,omitempty" validate:"required_without=Field,excluded_with=Field"` +} +``` + +### ValidationErrors + +Collect and manage multiple validation errors: + +```go +errors := &ValidationErrors{} +errors.Add("path.to.field", "error message") +errors.Extend(otherErrors) // Merge from another ValidationErrors + +if errors.HasErrors() { + fmt.Println(errors.First()) // Get first error message + fmt.Println(errors.Count()) // Number of errors + return errors // Implements error interface +} +``` ### CaptureField @@ -145,7 +225,6 @@ build: See `types.go` for complete definitions. - ## Related - `internal/criteria` - Evaluates conditions diff --git a/internal/config_loader/loader.go b/internal/config_loader/loader.go index 3be9c08..619e4be 100644 --- a/internal/config_loader/loader.go +++ b/internal/config_loader/loader.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "gopkg.in/yaml.v3" ) @@ -124,55 +125,142 @@ func Parse(data []byte, opts ...LoaderOption) (*AdapterConfig, error) { // Validation Pipeline // ----------------------------------------------------------------------------- -// validatorFunc is a function that validates a config and returns an error -type validatorFunc func(*AdapterConfig) error - // runValidationPipeline executes all validators in sequence func runValidationPipeline(config *AdapterConfig, cfg *loaderConfig) error { - // Core structural validators (always run) - coreValidators := []validatorFunc{ - validateAPIVersionAndKind, - validateMetadata, - validateAdapterSpec, - validateParams, - validatePreconditions, - validateResources, - validatePostActions, - validatePayloads, - } - - for _, v := range coreValidators { - if err := v(config); err != nil { - return fmt.Errorf("validation failed: %w", err) - } + validator := NewValidator(config, cfg.baseDir) + + // 1. Structural validation (fail-fast) + if err := validator.ValidateStructure(); err != nil { + return fmt.Errorf("validation failed: %w", err) } - // Adapter version validation (optional) + // 2. Adapter version validation (optional) if cfg.adapterVersion != "" { if err := ValidateAdapterVersion(config, cfg.adapterVersion); err != nil { return fmt.Errorf("adapter version validation failed: %w", err) } } - // File reference validation (buildRef, manifest.ref) - // Only run if baseDir is set (when loaded from file) + // 3. File reference validation and loading (only if baseDir is set) if cfg.baseDir != "" { - if err := validateFileReferences(config, cfg.baseDir); err != nil { + if err := validator.ValidateFileReferences(); err != nil { return fmt.Errorf("file reference validation failed: %w", err) } - // Load file references (manifest.ref, buildRef) after validation passes if err := loadFileReferences(config, cfg.baseDir); err != nil { return fmt.Errorf("failed to load file references: %w", err) } } - // Semantic validation (optional, can be skipped for performance) + // 4. Semantic validation (optional, can be skipped for performance) if !cfg.skipSemanticValidation { - if err := newValidator(config).Validate(); err != nil { + if err := validator.ValidateSemantic(); err != nil { return fmt.Errorf("semantic validation failed: %w", err) } } return nil } + +// ----------------------------------------------------------------------------- +// File Reference Loading +// ----------------------------------------------------------------------------- + +// loadFileReferences loads content from file references into the config +func loadFileReferences(config *AdapterConfig, baseDir string) error { + // Load manifest.ref or manifest.refs in spec.resources + for i := range config.Spec.Resources { + resource := &config.Spec.Resources[i] + refs := resource.GetManifestRefs() + if len(refs) == 0 { + continue + } + + // Load all referenced files + items := make([]map[string]interface{}, 0, len(refs)) + for j, ref := range refs { + content, err := loadYAMLFile(baseDir, ref) + if err != nil { + if len(refs) == 1 { + return fmt.Errorf("%s.%s[%d].%s.%s: %w", FieldSpec, FieldResources, i, FieldManifest, FieldRef, err) + } + return fmt.Errorf("%s.%s[%d].%s.%s[%d]: %w", FieldSpec, FieldResources, i, FieldManifest, FieldRefs, j, err) + } + items = append(items, content) + } + + // Store loaded items + if len(items) == 1 { + // Single ref: replace manifest with content (backward compatible) + resource.Manifest = items[0] + } else { + // Multiple refs: store in ManifestItems array + resource.ManifestItems = items + } + } + + // Load buildRef in spec.post.payloads + if config.Spec.Post != nil { + for i := range config.Spec.Post.Payloads { + payload := &config.Spec.Post.Payloads[i] + if payload.BuildRef != "" { + content, err := loadYAMLFile(baseDir, payload.BuildRef) + if err != nil { + return fmt.Errorf("%s.%s.%s[%d].%s: %w", FieldSpec, FieldPost, FieldPayloads, i, FieldBuildRef, err) + } + payload.BuildRefContent = content + } + } + } + + return nil +} + +// loadYAMLFile loads and parses a YAML file +func loadYAMLFile(baseDir, refPath string) (map[string]interface{}, error) { + fullPath, err := resolvePath(baseDir, refPath) + if err != nil { + return nil, err + } + + data, err := os.ReadFile(fullPath) + if err != nil { + return nil, fmt.Errorf("failed to read file %q: %w", fullPath, err) + } + + var content map[string]interface{} + if err := yaml.Unmarshal(data, &content); err != nil { + return nil, fmt.Errorf("failed to parse YAML file %q: %w", fullPath, err) + } + + return content, nil +} + +// resolvePath resolves a relative path against the base directory and validates +// that the resolved path does not escape the base directory. +func resolvePath(baseDir, refPath string) (string, error) { + baseAbs, err := filepath.Abs(baseDir) + if err != nil { + return "", fmt.Errorf("failed to resolve base directory: %w", err) + } + baseClean := filepath.Clean(baseAbs) + + var targetPath string + if filepath.IsAbs(refPath) { + targetPath = filepath.Clean(refPath) + } else { + targetPath = filepath.Clean(filepath.Join(baseClean, refPath)) + } + + // Check if target path is within base directory + rel, err := filepath.Rel(baseClean, targetPath) + if err != nil { + return "", fmt.Errorf("path %q escapes base directory", refPath) + } + + if strings.HasPrefix(rel, "..") { + return "", fmt.Errorf("path %q escapes base directory", refPath) + } + + return targetPath, nil +} diff --git a/internal/config_loader/loader_test.go b/internal/config_loader/loader_test.go index e772ea0..4a1d0f4 100644 --- a/internal/config_loader/loader_test.go +++ b/internal/config_loader/loader_test.go @@ -338,7 +338,7 @@ spec: - name: "checkCluster" `, wantError: true, - errorMsg: "must specify apiCall, expression, or conditions", + errorMsg: "must specify", }, { name: "API call without method", @@ -1184,7 +1184,7 @@ func TestValidateResourceDiscovery(t *testing.T) { }, }, wantErr: true, - errMsg: "must have either byName or bySelectors", + errMsg: "must have either", }, { name: "invalid - bySelectors without labelSelector defined", @@ -1205,7 +1205,7 @@ func TestValidateResourceDiscovery(t *testing.T) { }, }, wantErr: true, - errMsg: "must have labelSelector defined", + errMsg: "labelSelector", }, { name: "valid - manifest.refs array with discovery", diff --git a/internal/config_loader/struct_validator.go b/internal/config_loader/struct_validator.go new file mode 100644 index 0000000..c52af5c --- /dev/null +++ b/internal/config_loader/struct_validator.go @@ -0,0 +1,204 @@ +package config_loader + +import ( + "fmt" + "reflect" + "regexp" + "strings" + "sync" + + "github.com/go-playground/validator/v10" + + "github.com/openshift-hyperfleet/hyperfleet-adapter/internal/criteria" +) + +// ----------------------------------------------------------------------------- +// Struct Validator (go-playground/validator integration) +// ----------------------------------------------------------------------------- + +var ( + structValidator *validator.Validate + structValidatorOnce sync.Once + // fieldNameCache maps Go struct field names to yaml tag names (built via reflection) + fieldNameCache = make(map[string]string) +) + +// resourceNamePattern validates resource names for CEL compatibility. +// Must start with lowercase letter, can contain letters, numbers, underscores. +// Hyphens (kebab-case) are NOT allowed as they conflict with CEL's minus operator. +var resourceNamePattern = regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*$`) + +// extractYamlTagName extracts the yaml tag name from a struct field. +// Returns the Go field name if no yaml tag is defined. +func extractYamlTagName(fld reflect.StructField) string { + name := strings.SplitN(fld.Tag.Get("yaml"), ",", 2)[0] + if name == "-" || name == "" { + return fld.Name + } + return name +} + +// buildFieldNameCache recursively scans a type and caches Go field name -> yaml tag name mappings +func buildFieldNameCache(t reflect.Type, visited map[reflect.Type]bool) { + switch t.Kind() { + case reflect.Ptr: + buildFieldNameCache(t.Elem(), visited) + case reflect.Slice, reflect.Array, reflect.Map: + buildFieldNameCache(t.Elem(), visited) + case reflect.Struct: + if visited[t] { + return + } + visited[t] = true + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + fieldNameCache[field.Name] = extractYamlTagName(field) + buildFieldNameCache(field.Type, visited) + } + } +} + +// getStructValidator returns a singleton validator instance with custom validations registered +func getStructValidator() *validator.Validate { + structValidatorOnce.Do(func() { + structValidator = validator.New() + + // Register custom validations + _ = structValidator.RegisterValidation("resourcename", validateResourceName) + _ = structValidator.RegisterValidation("validoperator", validateOperator) + + // Use yaml tag names for field names in errors + structValidator.RegisterTagNameFunc(extractYamlTagName) + + // Build field name cache by reflecting on AdapterConfig + buildFieldNameCache(reflect.TypeOf(AdapterConfig{}), make(map[reflect.Type]bool)) + }) + return structValidator +} + +// validateResourceName is a custom validator for CEL-compatible resource names +func validateResourceName(fl validator.FieldLevel) bool { + return resourceNamePattern.MatchString(fl.Field().String()) +} + +// validateOperator is a custom validator for condition operators +func validateOperator(fl validator.FieldLevel) bool { + return criteria.IsValidOperator(fl.Field().String()) +} + +// ValidateStruct validates a struct using go-playground/validator tags. +// Returns a ValidationErrors with all validation failures. +func ValidateStruct(s interface{}) *ValidationErrors { + v := getStructValidator() + err := v.Struct(s) + if err == nil { + return nil + } + + validationErrors := &ValidationErrors{} + + if errs, ok := err.(validator.ValidationErrors); ok { + for _, e := range errs { + // Format as "path message" to match existing error format + msg := formatFullErrorMessage(e) + validationErrors.Add("", msg) + } + } else { + validationErrors.Add("", err.Error()) + } + + return validationErrors +} + +// formatFullErrorMessage creates a complete error message matching existing format +// e.g., "apiVersion is required" or "spec.adapter.version is required" +func formatFullErrorMessage(e validator.FieldError) string { + path := formatFieldPath(e.Namespace()) + field := e.Field() + + switch e.Tag() { + case "required": + return fmt.Sprintf("%s is required", path) + case "required_without": + // e.g., "field is required when expression is not set" + otherField := yamlFieldName(e.Param()) + return fmt.Sprintf("%s: must have either '%s' or '%s' set", parentPath(path), field, otherField) + case "excluded_with": + // e.g., "field and expression cannot both be set" + otherField := yamlFieldName(e.Param()) + return fmt.Sprintf("%s: '%s' and '%s' are mutually exclusive", parentPath(path), field, otherField) + case "eq": + return fmt.Sprintf("invalid %s %q (expected: %q)", path, e.Value(), e.Param()) + case "oneof": + return fmt.Sprintf("%s %q is invalid (allowed: %s)", path, e.Value(), strings.ReplaceAll(e.Param(), " ", ", ")) + case "resourcename": + return fmt.Sprintf("%s %q: must start with lowercase letter and contain only letters, numbers, underscores (no hyphens)", path, e.Value()) + case "validoperator": + return fmt.Sprintf("%s: invalid operator %q, must be one of: %s", path, e.Value(), strings.Join(criteria.OperatorStrings(), ", ")) + case "required_without_all": + // e.g., "must specify apiCall, expression, or conditions" + params := strings.Split(e.Param(), " ") + return fmt.Sprintf("%s: must specify %s or %s", parentPath(path), field, strings.Join(params, " or ")) + case "min": + return fmt.Sprintf("%s: must have at least %s element(s)", path, e.Param()) + default: + return fmt.Sprintf("%s: failed validation %s", path, e.Tag()) + } +} + +// parentPath returns the parent path (removes last segment) +// e.g., "spec.preconditions[0].capture[0].field" -> "spec.preconditions[0].capture[0]" +func parentPath(path string) string { + lastDot := strings.LastIndex(path, ".") + if lastDot == -1 { + return path + } + return path[:lastDot] +} + +// yamlFieldName returns the yaml tag name for a Go struct field name. +// Uses the cache built from reflecting on AdapterConfig. +// Falls back to lowercasing the first character if not in the cache. +func yamlFieldName(goFieldName string) string { + // Ensure cache is populated + getStructValidator() + + if yamlName, ok := fieldNameCache[goFieldName]; ok { + return yamlName + } + // Fallback: lowercase first character (common convention) + if goFieldName == "" { + return goFieldName + } + return strings.ToLower(goFieldName[:1]) + goFieldName[1:] +} + +// formatFieldPath converts validator namespace to our path format +// e.g., "AdapterConfig.Spec.Resources[0].Name" -> "spec.resources[0].name" +// Also handles embedded structs by removing the embedded type name +// e.g., "AdapterConfig.Spec.Preconditions[0].ActionBase.Name" -> "spec.preconditions[0].name" +func formatFieldPath(namespace string) string { + // Remove the root struct name (e.g., "AdapterConfig.") + parts := strings.SplitN(namespace, ".", 2) + if len(parts) < 2 { + return strings.ToLower(namespace) + } + path := parts[1] + + // Remove embedded struct names (they start with uppercase and don't have indices) + // e.g., "Preconditions[0].ActionBase.name" -> "Preconditions[0].name" + pathParts := strings.Split(path, ".") + var cleanParts []string + for _, part := range pathParts { + // Skip parts that are embedded struct names (uppercase, no index brackets) + if len(part) > 0 && part[0] >= 'A' && part[0] <= 'Z' && !strings.Contains(part, "[") { + continue + } + cleanParts = append(cleanParts, part) + } + + return strings.Join(cleanParts, ".") +} + + diff --git a/internal/config_loader/types.go b/internal/config_loader/types.go index 2b0bcac..22e6562 100644 --- a/internal/config_loader/types.go +++ b/internal/config_loader/types.go @@ -2,10 +2,21 @@ package config_loader import ( "fmt" + "strings" "gopkg.in/yaml.v3" ) +// FieldExpressionDef represents a common pattern for value extraction. +// Used when a value should be computed via field extraction (JSONPath) or CEL expression. +// Only one of Field or Expression should be set. +type FieldExpressionDef struct { + // Field uses JSONPath/dot notation to extract value (mutually exclusive with Expression) + Field string `yaml:"field,omitempty" validate:"required_without=Expression,excluded_with=Expression"` + // Expression uses CEL expression to evaluate (mutually exclusive with Field) + Expression string `yaml:"expression,omitempty" validate:"required_without=Field,excluded_with=Field"` +} + // ValueDef represents a dynamic value definition in payload builds. // Used when a payload field should be computed via field extraction (JSONPath) or CEL expression. // Only one of Field or Expression should be set. @@ -22,9 +33,8 @@ import ( // expression: "adapter.?errorMessage.orValue(\"\")" // default: "success" type ValueDef struct { - Field string `yaml:"field"` // JSONPath/dot notation to extract value - Expression string `yaml:"expression"` // CEL expression to evaluate - Default any `yaml:"default"` // Default value if extraction fails or returns nil + FieldExpressionDef `yaml:",inline"` + Default any `yaml:"default"` // Default value if extraction fails or returns nil } // ParseValueDef attempts to parse a value as a ValueDef. @@ -57,15 +67,15 @@ func ParseValueDef(v any) (*ValueDef, bool) { // AdapterConfig represents the complete adapter configuration structure type AdapterConfig struct { - APIVersion string `yaml:"apiVersion"` - Kind string `yaml:"kind"` + APIVersion string `yaml:"apiVersion" validate:"required"` + Kind string `yaml:"kind" validate:"required,eq=AdapterConfig"` Metadata Metadata `yaml:"metadata"` Spec AdapterConfigSpec `yaml:"spec"` } // Metadata contains the adapter metadata type Metadata struct { - Name string `yaml:"name"` + Name string `yaml:"name" validate:"required"` Namespace string `yaml:"namespace"` Labels map[string]string `yaml:"labels,omitempty"` } @@ -75,15 +85,15 @@ type AdapterConfigSpec struct { Adapter AdapterInfo `yaml:"adapter"` HyperfleetAPI HyperfleetAPIConfig `yaml:"hyperfleetApi"` Kubernetes KubernetesConfig `yaml:"kubernetes"` - Params []Parameter `yaml:"params,omitempty"` - Preconditions []Precondition `yaml:"preconditions,omitempty"` - Resources []Resource `yaml:"resources,omitempty"` - Post *PostConfig `yaml:"post,omitempty"` + Params []Parameter `yaml:"params,omitempty" validate:"dive"` + Preconditions []Precondition `yaml:"preconditions,omitempty" validate:"dive"` + Resources []Resource `yaml:"resources,omitempty" validate:"dive"` + Post *PostConfig `yaml:"post,omitempty" validate:"omitempty"` } // AdapterInfo contains basic adapter information type AdapterInfo struct { - Version string `yaml:"version"` + Version string `yaml:"version" validate:"required"` } // HyperfleetAPIConfig contains HyperFleet API configuration @@ -102,8 +112,8 @@ type KubernetesConfig struct { // Parameter represents a parameter extraction configuration. // Parameters are extracted from external sources (event data, env vars) using Source. type Parameter struct { - Name string `yaml:"name"` - Source string `yaml:"source,omitempty"` + Name string `yaml:"name" validate:"required"` + Source string `yaml:"source,omitempty" validate:"required"` Type string `yaml:"type,omitempty"` Description string `yaml:"description,omitempty"` Required bool `yaml:"required,omitempty"` @@ -118,49 +128,39 @@ type Parameter struct { // - Use Build for inline payload definitions directly in the config // - Use BuildRef to reference an external YAML file containing the build definition type Payload struct { - Name string `yaml:"name"` + Name string `yaml:"name" validate:"required"` // Build contains a structure that will be evaluated and converted to JSON at runtime. // The structure is kept as raw interface{} to allow flexible schema definitions. // Mutually exclusive with BuildRef. - Build interface{} `yaml:"build,omitempty"` + Build interface{} `yaml:"build,omitempty" validate:"required_without=BuildRef,excluded_with=BuildRef"` // BuildRef references an external YAML file containing the build definition. // Mutually exclusive with Build. - BuildRef string `yaml:"buildRef,omitempty"` + BuildRef string `yaml:"buildRef,omitempty" validate:"required_without=Build,excluded_with=Build"` // BuildRefContent holds the loaded content from BuildRef file (populated by loader) BuildRefContent map[string]interface{} `yaml:"-"` } -// Validate checks that the Payload configuration is valid. -// Returns an error if: -// - Both Build and BuildRef are set (mutually exclusive) -// - Neither Build nor BuildRef is set (one is required) -func (p *Payload) Validate() error { - hasBuild := p.Build != nil - hasBuildRef := p.BuildRef != "" - - if hasBuild && hasBuildRef { - return fmt.Errorf("build and buildRef are mutually exclusive; set only one") - } - if !hasBuild && !hasBuildRef { - return fmt.Errorf("either build or buildRef must be set") - } - return nil +// ActionBase contains common fields for action-like configurations. +// Used by Precondition and PostAction to reduce duplication. +type ActionBase struct { + Name string `yaml:"name" validate:"required"` + APICall *APICall `yaml:"apiCall,omitempty" validate:"omitempty"` + Log *LogAction `yaml:"log,omitempty"` } -// Precondition represents a precondition check +// Precondition represents a precondition check. +// Must have at least one of: APICall (from ActionBase), Expression, or Conditions. type Precondition struct { - Name string `yaml:"name"` - APICall *APICall `yaml:"apiCall,omitempty"` - Capture []CaptureField `yaml:"capture,omitempty"` - Conditions []Condition `yaml:"conditions,omitempty"` - Expression string `yaml:"expression,omitempty"` - Log *LogAction `yaml:"log,omitempty"` + ActionBase `yaml:",inline"` + Capture []CaptureField `yaml:"capture,omitempty" validate:"dive"` + Conditions []Condition `yaml:"conditions,omitempty" validate:"dive,required_without_all=ActionBase.APICall Expression"` + Expression string `yaml:"expression,omitempty" validate:"required_without_all=ActionBase.APICall Conditions"` } // APICall represents an API call configuration type APICall struct { - Method string `yaml:"method"` - URL string `yaml:"url"` + Method string `yaml:"method" validate:"required,oneof=GET POST PUT PATCH DELETE"` + URL string `yaml:"url" validate:"required"` Timeout string `yaml:"timeout,omitempty"` RetryAttempts int `yaml:"retryAttempts,omitempty"` RetryBackoff string `yaml:"retryBackoff,omitempty"` @@ -180,15 +180,14 @@ type Header struct { // - Field: JSONPath expression for simple field extraction (e.g., "{.items[0].name}") // - Expression: CEL expression for complex transformations (e.g., "response.items.filter(i, i.adapter == 'x')") type CaptureField struct { - Name string `yaml:"name"` - Field string `yaml:"field,omitempty"` - Expression string `yaml:"expression,omitempty"` + Name string `yaml:"name" validate:"required"` + FieldExpressionDef `yaml:",inline"` } // Condition represents a structured condition type Condition struct { Field string `yaml:"field"` - Operator string `yaml:"operator"` + Operator string `yaml:"operator" validate:"required,validoperator"` Value interface{} `yaml:"-"` // Populated by UnmarshalYAML from "value" or "values" } @@ -227,10 +226,10 @@ func (c *Condition) UnmarshalYAML(unmarshal func(interface{}) error) error { // Resource represents a Kubernetes resource configuration type Resource struct { - Name string `yaml:"name"` - Manifest interface{} `yaml:"manifest,omitempty"` + Name string `yaml:"name" validate:"required,resourcename"` + Manifest interface{} `yaml:"manifest,omitempty" validate:"required"` RecreateOnChange bool `yaml:"recreateOnChange,omitempty"` - Discovery *DiscoveryConfig `yaml:"discovery,omitempty"` + Discovery *DiscoveryConfig `yaml:"discovery,omitempty" validate:"required"` // ManifestItems holds loaded content when manifest.ref is an array (populated by loader) ManifestItems []map[string]interface{} `yaml:"-"` } @@ -238,26 +237,24 @@ type Resource struct { // DiscoveryConfig represents resource discovery configuration type DiscoveryConfig struct { Namespace string `yaml:"namespace,omitempty"` - ByName string `yaml:"byName,omitempty"` - BySelectors *SelectorConfig `yaml:"bySelectors,omitempty"` + ByName string `yaml:"byName,omitempty" validate:"required_without=BySelectors"` + BySelectors *SelectorConfig `yaml:"bySelectors,omitempty" validate:"required_without=ByName,omitempty"` } // SelectorConfig represents label selector configuration type SelectorConfig struct { - LabelSelector map[string]string `yaml:"labelSelector,omitempty"` + LabelSelector map[string]string `yaml:"labelSelector,omitempty" validate:"required,min=1"` } // PostConfig represents post-processing configuration type PostConfig struct { - Payloads []Payload `yaml:"payloads,omitempty"` - PostActions []PostAction `yaml:"postActions,omitempty"` + Payloads []Payload `yaml:"payloads,omitempty" validate:"dive"` + PostActions []PostAction `yaml:"postActions,omitempty" validate:"dive"` } // PostAction represents a post-processing action type PostAction struct { - Name string `yaml:"name"` - APICall *APICall `yaml:"apiCall,omitempty"` - Log *LogAction `yaml:"log,omitempty"` + ActionBase `yaml:",inline"` } // LogAction represents a logging action that can be configured in the adapter config @@ -270,3 +267,61 @@ type LogAction struct { type ManifestRef struct { Ref string `yaml:"ref"` } + +// ----------------------------------------------------------------------------- +// Validation Errors +// ----------------------------------------------------------------------------- + +// ValidationError represents a validation error with context +type ValidationError struct { + Path string + Message string +} + +func (e *ValidationError) Error() string { + return fmt.Sprintf("%s: %s", e.Path, e.Message) +} + +// ValidationErrors holds multiple validation errors +type ValidationErrors struct { + Errors []ValidationError +} + +func (ve *ValidationErrors) Error() string { + if len(ve.Errors) == 0 { + return "no validation errors" + } + var msgs []string + for _, e := range ve.Errors { + msgs = append(msgs, e.Error()) + } + return fmt.Sprintf("validation failed with %d error(s):\n - %s", len(ve.Errors), strings.Join(msgs, "\n - ")) +} + +func (ve *ValidationErrors) Add(path, message string) { + ve.Errors = append(ve.Errors, ValidationError{Path: path, Message: message}) +} + +// Extend appends all errors from another ValidationErrors +func (ve *ValidationErrors) Extend(other *ValidationErrors) { + if other != nil { + ve.Errors = append(ve.Errors, other.Errors...) + } +} + +// First returns the first error message, or empty string if no errors +func (ve *ValidationErrors) First() string { + if len(ve.Errors) == 0 { + return "" + } + return ve.Errors[0].Message +} + +// Count returns the number of errors +func (ve *ValidationErrors) Count() int { + return len(ve.Errors) +} + +func (ve *ValidationErrors) HasErrors() bool { + return len(ve.Errors) > 0 +} diff --git a/internal/config_loader/validator.go b/internal/config_loader/validator.go index 54ce732..ba5652e 100644 --- a/internal/config_loader/validator.go +++ b/internal/config_loader/validator.go @@ -2,6 +2,7 @@ package config_loader import ( "fmt" + "os" "reflect" "regexp" "strings" @@ -11,80 +12,82 @@ import ( "github.com/openshift-hyperfleet/hyperfleet-adapter/internal/criteria" ) +// templateVarRegex matches Go template variables like {{ .varName }} or {{ .nested.var }} +var templateVarRegex = regexp.MustCompile(`\{\{\s*\.([a-zA-Z_][a-zA-Z0-9_\.]*)\s*(?:\|[^}]*)?\}\}`) + // ----------------------------------------------------------------------------- -// Validation Errors +// Validator // ----------------------------------------------------------------------------- -// ValidationError represents a validation error with context -type ValidationError struct { - Path string - Message string -} - -func (e *ValidationError) Error() string { - return fmt.Sprintf("%s: %s", e.Path, e.Message) -} - -// ValidationErrors holds multiple validation errors -type ValidationErrors struct { - Errors []ValidationError +// Validator performs all validation on AdapterConfig: +// - Structural validation (required fields, formats) +// - Semantic validation (CEL expressions, templates, operators, K8s manifests) +type Validator struct { + config *AdapterConfig + baseDir string // Base directory for file reference validation + errors *ValidationErrors + definedVars map[string]bool // All defined variables (params + resources + captures) + celEnv *cel.Env } -func (ve *ValidationErrors) Error() string { - if len(ve.Errors) == 0 { - return "no validation errors" - } - var msgs []string - for _, e := range ve.Errors { - msgs = append(msgs, e.Error()) +// NewValidator creates a new Validator for the given config +func NewValidator(config *AdapterConfig, baseDir string) *Validator { + return &Validator{ + config: config, + baseDir: baseDir, + errors: &ValidationErrors{}, } - return fmt.Sprintf("validation failed with %d error(s):\n - %s", len(ve.Errors), strings.Join(msgs, "\n - ")) } -func (ve *ValidationErrors) Add(path, message string) { - ve.Errors = append(ve.Errors, ValidationError{Path: path, Message: message}) -} +// ValidateStructure performs structural validation (required fields, formats). +// Uses go-playground/validator for basic struct validation, then runs custom validations. +// Returns error on first failure (fail-fast). +func (v *Validator) ValidateStructure() error { + // Phase 1: Struct tag validation (required fields, formats, enums) + if errs := ValidateStruct(v.config); errs != nil && errs.HasErrors() { + return fmt.Errorf("%s", errs.First()) + } -func (ve *ValidationErrors) HasErrors() bool { - return len(ve.Errors) > 0 -} + // Phase 2: Custom validations that can't be expressed via struct tags + customValidators := []func() error{ + v.validateAPIVersionSupported, // Check apiVersion is in supported list + v.validateEnvParams, // Check required env params have values + v.validateDuplicateResourceNames, // Check for duplicate resource names + } -// ----------------------------------------------------------------------------- -// Validator -// ----------------------------------------------------------------------------- + for _, validate := range customValidators { + if err := validate(); err != nil { + return err + } + } -// Validator performs semantic validation on AdapterConfig. -// It validates operators, template variables, CEL expressions, and K8s manifests. -type Validator struct { - config *AdapterConfig - errors *ValidationErrors - definedParams map[string]bool - celEnv *cel.Env + return nil } -// newValidator creates a new Validator for the given config -func newValidator(config *AdapterConfig) *Validator { - return &Validator{ - config: config, - errors: &ValidationErrors{}, +// ValidateFileReferences validates that all file references exist. +// Only meaningful if baseDir is set. +func (v *Validator) ValidateFileReferences() error { + if v.baseDir == "" { + return nil } + return v.validateFileReferences() } -// Validate performs all semantic validations and returns any errors. -// This is the main entry point for validation. -func (v *Validator) Validate() error { +// ValidateSemantic performs semantic validation (CEL, templates, operators, K8s). +// Collects all errors rather than failing fast. +func (v *Validator) ValidateSemantic() error { if v.config == nil { return fmt.Errorf("config is nil") } // Initialize validation context - v.collectDefinedParameters() + v.collectDefinedVariables() if err := v.initCELEnv(); err != nil { v.errors.Add("cel", fmt.Sprintf("failed to create CEL environment: %v", err)) } - // Run all validators - v.validateConditionOperators() + // Run all semantic validators + v.validateConditionValues() v.validateCaptureFields() v.validateTemplateVariables() v.validateCELExpressions() @@ -96,67 +99,150 @@ func (v *Validator) Validate() error { return nil } -// ----------------------------------------------------------------------------- -// Parameter Collection -// ----------------------------------------------------------------------------- +// ============================================================================= +// CUSTOM STRUCTURAL VALIDATION +// ============================================================================= +// These validations can't be expressed via struct tags and run after tag validation. -// collectDefinedParameters collects all defined parameter names for template validation -func (v *Validator) collectDefinedParameters() { - v.definedParams = v.config.GetDefinedVariables() +// validateAPIVersionSupported checks that the apiVersion is in the supported list +func (v *Validator) validateAPIVersionSupported() error { + if !IsSupportedAPIVersion(v.config.APIVersion) { + return fmt.Errorf("unsupported apiVersion %q (supported: %s)", + v.config.APIVersion, strings.Join(SupportedAPIVersions, ", ")) + } + return nil } -// ----------------------------------------------------------------------------- -// Operator Validation -// ----------------------------------------------------------------------------- +// validateEnvParams checks that required env params have values set +func (v *Validator) validateEnvParams() error { + for i, param := range v.config.Spec.Params { + if param.Required && strings.HasPrefix(param.Source, "env.") { + envName := strings.TrimPrefix(param.Source, "env.") + envValue := os.Getenv(envName) + if envValue == "" && param.Default == nil { + path := fmt.Sprintf("%s.%s[%d]", FieldSpec, FieldParams, i) + return fmt.Errorf("%s (%s): required environment variable %s is not set", path, param.Name, envName) + } + } + } + return nil +} -// validateConditionOperators validates all condition operators in the config -func (v *Validator) validateConditionOperators() { - // Validate precondition conditions - for i, precond := range v.config.Spec.Preconditions { - for j, cond := range precond.Conditions { - path := fmt.Sprintf("%s.%s[%d].%s[%d]", FieldSpec, FieldPreconditions, i, FieldConditions, j) - v.validateCondition(cond, path) +// validateDuplicateResourceNames checks for duplicate resource names +func (v *Validator) validateDuplicateResourceNames() error { + seen := make(map[string]bool) + + for i, resource := range v.config.Spec.Resources { + if seen[resource.Name] { + path := fmt.Sprintf("%s.%s[%d]", FieldSpec, FieldResources, i) + return fmt.Errorf("%s.%s %q: duplicate resource name", path, FieldName, resource.Name) } + seen[resource.Name] = true } + return nil } -// validateCondition validates a single condition including operator and value -func (v *Validator) validateCondition(cond Condition, path string) { - // Validate operator - v.validateOperator(cond.Operator, path) +// ============================================================================= +// FILE REFERENCE VALIDATION +// ============================================================================= + +func (v *Validator) validateFileReferences() error { + var errors []string + + // Validate buildRef in spec.post.payloads + if v.config.Spec.Post != nil { + for i, payload := range v.config.Spec.Post.Payloads { + if payload.BuildRef != "" { + path := fmt.Sprintf("%s.%s.%s[%d].%s", FieldSpec, FieldPost, FieldPayloads, i, FieldBuildRef) + if err := v.validateFileExists(payload.BuildRef, path); err != nil { + errors = append(errors, err.Error()) + } + } + } + } + + // Validate manifest.ref and manifest.refs in spec.resources + for i, resource := range v.config.Spec.Resources { + refs := resource.GetManifestRefs() + for j, ref := range refs { + var path string + if len(refs) == 1 { + path = fmt.Sprintf("%s.%s[%d].%s.%s", FieldSpec, FieldResources, i, FieldManifest, FieldRef) + } else { + path = fmt.Sprintf("%s.%s[%d].%s.%s[%d]", FieldSpec, FieldResources, i, FieldManifest, FieldRefs, j) + } + if err := v.validateFileExists(ref, path); err != nil { + errors = append(errors, err.Error()) + } + } + } - // Validate value based on operator - v.validateConditionValue(cond.Operator, cond.Value, path) + if len(errors) > 0 { + return fmt.Errorf("file reference errors:\n - %s", strings.Join(errors, "\n - ")) + } + return nil } -// validateOperator checks if an operator is valid -func (v *Validator) validateOperator(operator string, path string) { - if operator == "" { - v.errors.Add(path, "operator is required") - return +func (v *Validator) validateFileExists(refPath, configPath string) error { + if refPath == "" { + return fmt.Errorf("%s: file reference is empty", configPath) + } + + fullPath, err := resolvePath(v.baseDir, refPath) + if err != nil { + return fmt.Errorf("%s: %w", configPath, err) + } + + info, err := os.Stat(fullPath) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("%s: referenced file %q does not exist (resolved to %q)", configPath, refPath, fullPath) + } + return fmt.Errorf("%s: error checking file %q: %w", configPath, refPath, err) + } + + if info.IsDir() { + return fmt.Errorf("%s: referenced path %q is a directory, not a file", configPath, refPath) } - if !criteria.IsValidOperator(operator) { - v.errors.Add(path, fmt.Sprintf("invalid operator %q, must be one of: %s", - operator, strings.Join(criteria.OperatorStrings(), ", "))) + + return nil +} + +// ============================================================================= +// SEMANTIC VALIDATION: Operators +// ============================================================================= + +func (v *Validator) collectDefinedVariables() { + v.definedVars = v.config.GetDefinedVariables() +} + +func (v *Validator) validateConditionValues() { + // Operator validation (required, valid enum) is handled by struct validator. + // This validates condition values based on operator requirements. + for i, precond := range v.config.Spec.Preconditions { + for j, cond := range precond.Conditions { + path := fmt.Sprintf("%s.%s[%d].%s[%d]", FieldSpec, FieldPreconditions, i, FieldConditions, j) + v.validateConditionValue(cond.Operator, cond.Value, path) + } } } -// validateConditionValue validates that the value is appropriate for the operator func (v *Validator) validateConditionValue(operator string, value interface{}, path string) { op := criteria.Operator(operator) - // "exists" operator does not require a value if op == criteria.OperatorExists { + // "exists" operator checks for field presence, value should not be set + if value != nil { + v.errors.Add(path, fmt.Sprintf("value/values should not be set for operator \"%s\"", operator)) + } return } - // All other operators require a value if value == nil { v.errors.Add(path, fmt.Sprintf("value is required for operator %q", operator)) return } - // "in" and "notIn" operators require a list/array value if op == criteria.OperatorIn || op == criteria.OperatorNotIn { if !isSliceOrArray(value) { v.errors.Add(path, fmt.Sprintf("value must be a list for operator %q", operator)) @@ -164,7 +250,6 @@ func (v *Validator) validateConditionValue(operator string, value interface{}, p } } -// isSliceOrArray checks if a value is a slice or array func isSliceOrArray(value interface{}) bool { if value == nil { return false @@ -173,51 +258,32 @@ func isSliceOrArray(value interface{}) bool { return kind == reflect.Slice || kind == reflect.Array } -// ----------------------------------------------------------------------------- -// Capture Field Validation -// ----------------------------------------------------------------------------- +// ============================================================================= +// SEMANTIC VALIDATION: Capture Fields +// ============================================================================= -// validateCaptureFields validates capture fields in preconditions func (v *Validator) validateCaptureFields() { for i, precond := range v.config.Spec.Preconditions { for j, capture := range precond.Capture { path := fmt.Sprintf("%s.%s[%d].%s[%d]", FieldSpec, FieldPreconditions, i, FieldCapture, j) - v.validateCaptureField(capture, path) + v.validateCaptureFieldExpression(capture, path) } } } -// validateCaptureField validates a single capture field configuration -func (v *Validator) validateCaptureField(capture CaptureField, path string) { - // Name is required - if capture.Name == "" { - v.errors.Add(path, "capture name is required") - } - - hasField := capture.Field != "" - hasExpression := capture.Expression != "" - - // Must have exactly one of field or expression - if !hasField && !hasExpression { - v.errors.Add(path, "capture must have either 'field' or 'expression' set") - } else if hasField && hasExpression { - v.errors.Add(path, "capture cannot have both 'field' and 'expression' set; use only one") - } - - // If expression is set, validate it as CEL - if hasExpression && v.celEnv != nil { +func (v *Validator) validateCaptureFieldExpression(capture CaptureField, path string) { + // Structural validation (name required, field/expression mutual exclusivity) + // is handled by struct validator tags in types.go. + // This validates CEL expression syntax. + if capture.Expression != "" && v.celEnv != nil { v.validateCELExpression(capture.Expression, path+"."+FieldExpression) } } -// ----------------------------------------------------------------------------- -// Template Variable Validation -// ----------------------------------------------------------------------------- - -// templateVarRegex matches Go template variables like {{ .varName }} or {{ .nested.var }} -var templateVarRegex = regexp.MustCompile(`\{\{\s*\.([a-zA-Z_][a-zA-Z0-9_\.]*)\s*(?:\|[^}]*)?\}\}`) +// ============================================================================= +// SEMANTIC VALIDATION: Template Variables +// ============================================================================= -// validateTemplateVariables validates that template variables are defined func (v *Validator) validateTemplateVariables() { // Validate precondition API call URLs and bodies for i, precond := range v.config.Spec.Preconditions { @@ -265,7 +331,7 @@ func (v *Validator) validateTemplateVariables() { } } - // Validate post payload build value templates (build is now interface{}) + // Validate post payload build value templates for i, payload := range v.config.Spec.Post.Payloads { if payload.Build != nil { if buildMap, ok := payload.Build.(map[string]interface{}); ok { @@ -274,10 +340,8 @@ func (v *Validator) validateTemplateVariables() { } } } - } -// validateTemplateString checks template variables in a string func (v *Validator) validateTemplateString(s string, path string) { if s == "" { return @@ -294,30 +358,22 @@ func (v *Validator) validateTemplateString(s string, path string) { } } -// isVariableDefined checks if a variable is defined (including nested paths) func (v *Validator) isVariableDefined(varName string) bool { - // Check exact match - if v.definedParams[varName] { + if v.definedVars[varName] { return true } - // Check if the root variable is defined (for nested paths like clusterDetails.status.phase) parts := strings.Split(varName, ".") if len(parts) > 0 { root := parts[0] - // Handle simple root variables (e.g. "metadata", "clusterId") - if v.definedParams[root] { + if v.definedVars[root] { return true } - // Special handling for resource aliases: treat "resources." as a root. - // Resource aliases are registered as "resources.clusterNamespace" etc., - // so we need to check if "resources." is defined for paths like - // "resources.clusterNamespace.metadata.namespace" if root == FieldResources && len(parts) > 1 { alias := root + "." + parts[1] - if v.definedParams[alias] { + if v.definedVars[alias] { return true } } @@ -326,7 +382,6 @@ func (v *Validator) isVariableDefined(varName string) bool { return false } -// validateTemplateMap recursively validates template variables in a map func (v *Validator) validateTemplateMap(m map[string]interface{}, path string) { for key, value := range m { currentPath := fmt.Sprintf("%s.%s", path, key) @@ -348,45 +403,34 @@ func (v *Validator) validateTemplateMap(m map[string]interface{}, path string) { } } -// ----------------------------------------------------------------------------- -// CEL Expression Validation -// ----------------------------------------------------------------------------- +// ============================================================================= +// SEMANTIC VALIDATION: CEL Expressions +// ============================================================================= -// initCELEnv initializes the CEL environment dynamically from config-defined variables. -// This uses v.definedParams which must be populated by collectDefinedParameters() first. func (v *Validator) initCELEnv() error { - // Pre-allocate capacity: +2 for cel.OptionalTypes() and potential "resources" variable - options := make([]cel.EnvOption, 0, len(v.definedParams)+2) - - // Enable optional types for optional chaining syntax (e.g., a.?b.?c) + options := make([]cel.EnvOption, 0, len(v.definedVars)+2) options = append(options, cel.OptionalTypes()) - // Track root variables we've already added (to avoid duplicates for nested paths) addedRoots := make(map[string]bool) - for varName := range v.definedParams { - // Extract root variable name (e.g., "clusterDetails" from "clusterDetails.status.phase") + for varName := range v.definedVars { root := varName if idx := strings.Index(varName, "."); idx > 0 { root = varName[:idx] } - // Skip if we've already added this root variable if addedRoots[root] { continue } addedRoots[root] = true - // Use DynType since we don't know the actual type at validation time options = append(options, cel.Variable(root, cel.DynType)) } - // Always add "resources" as a map for resource lookups like resources.clusterNamespace if !addedRoots[FieldResources] { options = append(options, cel.Variable(FieldResources, cel.MapType(cel.StringType, cel.DynType))) } - // Always add "adapter" as a map for adapter metadata lookups like adapter.executionStatus if !addedRoots[FieldAdapter] { options = append(options, cel.Variable(FieldAdapter, cel.MapType(cel.StringType, cel.DynType))) } @@ -399,13 +443,11 @@ func (v *Validator) initCELEnv() error { return nil } -// validateCELExpressions validates all CEL expressions in the config func (v *Validator) validateCELExpressions() { if v.celEnv == nil { - return // CEL env initialization failed, already reported + return } - // Validate precondition expressions for i, precond := range v.config.Spec.Preconditions { if precond.Expression != "" { path := fmt.Sprintf("%s.%s[%d].%s", FieldSpec, FieldPreconditions, i, FieldExpression) @@ -413,8 +455,6 @@ func (v *Validator) validateCELExpressions() { } } - // Validate post payload build expressions (build is now interface{}) - // We recursively find and validate any "expression" fields in the build structure if v.config.Spec.Post != nil { for i, payload := range v.config.Spec.Post.Payloads { if payload.Build != nil { @@ -426,32 +466,24 @@ func (v *Validator) validateCELExpressions() { } } -// validateCELExpression validates a single CEL expression (syntax only) -// Type checking is skipped because variables are dynamic (DynType) and -// their actual types are only known at runtime. func (v *Validator) validateCELExpression(expr string, path string) { if expr == "" { return } - // Clean up the expression (remove leading/trailing whitespace and newlines) expr = strings.TrimSpace(expr) - // Syntax validation only _, issues := v.celEnv.Parse(expr) if issues != nil && issues.Err() != nil { v.errors.Add(path, fmt.Sprintf("CEL parse error: %v", issues.Err())) } } -// validateBuildExpressions recursively validates CEL expressions in a build structure. -// It looks for any field named "expression" and validates it as a CEL expression. func (v *Validator) validateBuildExpressions(m map[string]interface{}, path string) { for key, value := range m { currentPath := fmt.Sprintf("%s.%s", path, key) switch val := value.(type) { case string: - // If the key is "expression", validate it as CEL if key == FieldExpression { v.validateCELExpression(val, currentPath) } @@ -468,42 +500,33 @@ func (v *Validator) validateBuildExpressions(m map[string]interface{}, path stri } } -// ----------------------------------------------------------------------------- -// Kubernetes Manifest Validation -// ----------------------------------------------------------------------------- +// ============================================================================= +// SEMANTIC VALIDATION: Kubernetes Manifests +// ============================================================================= -// validateK8sManifests validates Kubernetes resource manifests func (v *Validator) validateK8sManifests() { for i, resource := range v.config.Spec.Resources { path := fmt.Sprintf("%s.%s[%d].%s", FieldSpec, FieldResources, i, FieldManifest) - // Validate inline or single-ref manifest if manifest, ok := resource.Manifest.(map[string]interface{}); ok { - // Check for ref (external template reference) if ref, hasRef := manifest[FieldRef].(string); hasRef { if ref == "" { v.errors.Add(path+"."+FieldRef, "manifest ref cannot be empty") } - // Single ref: content will have been loaded into Manifest by loadFileReferences - // and will be validated below if it's a valid manifest map } else if _, hasRefs := manifest[FieldRefs]; hasRefs { - // Multiple refs: content loaded into ManifestItems, validated below + // Multiple refs: validated via ManifestItems } else { - // Inline manifest - validate it v.validateK8sManifest(manifest, path) } } - // Validate any loaded manifest items from manifest.refs (multiple refs) for j, item := range resource.ManifestItems { v.validateK8sManifest(item, fmt.Sprintf("%s.%s[%d].%s[%d]", FieldSpec, FieldResources, i, FieldManifestItems, j)) } } } -// validateK8sManifest validates a single Kubernetes manifest func (v *Validator) validateK8sManifest(manifest map[string]interface{}, path string) { - // Required fields for K8s resources requiredFields := []string{FieldAPIVersion, FieldKind, FieldMetadata} for _, field := range requiredFields { @@ -512,24 +535,86 @@ func (v *Validator) validateK8sManifest(manifest map[string]interface{}, path st } } - // Validate metadata has name if metadata, ok := manifest[FieldMetadata].(map[string]interface{}); ok { if _, hasName := metadata[FieldName]; !hasName { v.errors.Add(path+"."+FieldMetadata, fmt.Sprintf("missing required field %q", FieldName)) } } - // Validate apiVersion format if apiVersion, ok := manifest[FieldAPIVersion].(string); ok { if apiVersion == "" { v.errors.Add(path+"."+FieldAPIVersion, "apiVersion cannot be empty") } } - // Validate kind if kind, ok := manifest[FieldKind].(string); ok { if kind == "" { v.errors.Add(path+"."+FieldKind, "kind cannot be empty") } } } + +// ============================================================================= +// HELPER FUNCTIONS +// ============================================================================= + +// IsSupportedAPIVersion checks if the given apiVersion is supported +func IsSupportedAPIVersion(apiVersion string) bool { + for _, v := range SupportedAPIVersions { + if v == apiVersion { + return true + } + } + return false +} + +// ValidateAdapterVersion validates the config's adapter version matches the expected version +func ValidateAdapterVersion(config *AdapterConfig, expectedVersion string) error { + if expectedVersion == "" { + return nil + } + + configVersion := config.Spec.Adapter.Version + if configVersion != expectedVersion { + return fmt.Errorf("adapter version mismatch: config %q != adapter %q", + configVersion, expectedVersion) + } + + return nil +} + +// ============================================================================= +// BACKWARD COMPATIBILITY HELPERS +// ============================================================================= + +// newValidator is a backward-compatible alias for NewValidator (for tests) +func newValidator(config *AdapterConfig) *Validator { + return NewValidator(config, "") +} + +// Validate is a convenience method that runs semantic validation +// (backward compatible with old Validator.Validate() interface) +func (v *Validator) Validate() error { + return v.ValidateSemantic() +} + +// validateFileReferences is a package-level helper for backward compatibility +func validateFileReferences(config *AdapterConfig, baseDir string) error { + return NewValidator(config, baseDir).ValidateFileReferences() +} + +// validateResources is a package-level helper for backward compatibility +func validateResources(config *AdapterConfig) error { + v := NewValidator(config, "") + // Run struct validation for resources first + if errs := ValidateStruct(config); errs != nil && errs.HasErrors() { + // Filter for resource-related errors + for _, e := range errs.Errors { + if strings.Contains(e.Message, "resources") || strings.Contains(e.Message, "Resources") { + return fmt.Errorf("%s", e.Message) + } + } + } + // Then check for duplicate resource names + return v.validateDuplicateResourceNames() +} diff --git a/internal/config_loader/validator_schema.go b/internal/config_loader/validator_schema.go deleted file mode 100644 index bddeeec..0000000 --- a/internal/config_loader/validator_schema.go +++ /dev/null @@ -1,545 +0,0 @@ -package config_loader - -import ( - "fmt" - "os" - "path/filepath" - "regexp" - "slices" - "strings" - - "gopkg.in/yaml.v3" -) - -// validResourceNameRegex validates resource names for CEL compatibility. -// Allows snake_case (my_resource) and camelCase (myResource). -// Must start with lowercase letter, can contain letters, numbers, underscores. -// Hyphens (kebab-case) are NOT allowed as they conflict with CEL's minus operator. -var validResourceNameRegex = regexp.MustCompile(`^[a-z][a-zA-Z0-9_]*$`) - -// ----------------------------------------------------------------------------- -// SchemaValidator -// ----------------------------------------------------------------------------- - -// SchemaValidator performs schema validation on AdapterConfig. -// It validates required fields, file references, and loads external files. -type SchemaValidator struct { - config *AdapterConfig - baseDir string // Base directory for resolving relative paths -} - -// NewSchemaValidator creates a new SchemaValidator for the given config -func NewSchemaValidator(config *AdapterConfig, baseDir string) *SchemaValidator { - return &SchemaValidator{ - config: config, - baseDir: baseDir, - } -} - -// ValidateStructure performs all structural validations. -// Returns error on first validation failure (fail-fast). -func (v *SchemaValidator) ValidateStructure() error { - validators := []func() error{ - v.validateAPIVersionAndKind, - v.validateMetadata, - v.validateAdapterSpec, - v.validateParams, - v.validatePreconditions, - v.validateResources, - v.validatePostActions, - v.validatePayloads, - } - - for _, validate := range validators { - if err := validate(); err != nil { - return err - } - } - - return nil -} - -// ValidateFileReferences validates that all file references exist. -// Only runs if baseDir is set. -func (v *SchemaValidator) ValidateFileReferences() error { - if v.baseDir == "" { - return nil - } - return v.validateFileReferences() -} - -// LoadFileReferences loads content from file references into the config. -// Only runs if baseDir is set. -func (v *SchemaValidator) LoadFileReferences() error { - if v.baseDir == "" { - return nil - } - return v.loadFileReferences() -} - -// ----------------------------------------------------------------------------- -// Core Structural Validators -// ----------------------------------------------------------------------------- - -func (v *SchemaValidator) validateAPIVersionAndKind() error { - if v.config.APIVersion == "" { - return fmt.Errorf("apiVersion is required") - } - if !IsSupportedAPIVersion(v.config.APIVersion) { - return fmt.Errorf("unsupported apiVersion %q (supported: %s)", - v.config.APIVersion, strings.Join(SupportedAPIVersions, ", ")) - } - if v.config.Kind == "" { - return fmt.Errorf("kind is required") - } - if v.config.Kind != ExpectedKind { - return fmt.Errorf("invalid kind %q (expected: %q)", v.config.Kind, ExpectedKind) - } - return nil -} - -func (v *SchemaValidator) validateMetadata() error { - if v.config.Metadata.Name == "" { - return fmt.Errorf("metadata.name is required") - } - return nil -} - -func (v *SchemaValidator) validateAdapterSpec() error { - if v.config.Spec.Adapter.Version == "" { - return fmt.Errorf("%s.%s.%s is required", FieldSpec, FieldAdapter, FieldVersion) - } - return nil -} - -func (v *SchemaValidator) validateParams() error { - for i, param := range v.config.Spec.Params { - path := fmt.Sprintf("%s.%s[%d]", FieldSpec, FieldParams, i) - - if param.Name == "" { - return fmt.Errorf("%s.%s is required", path, FieldName) - } - - if param.Source == "" { - return fmt.Errorf("%s (%s): %s is required", path, param.Name, FieldSource) - } - - // Validate required env params have values - if param.Required && strings.HasPrefix(param.Source, "env.") { - envName := strings.TrimPrefix(param.Source, "env.") - envValue := os.Getenv(envName) - if envValue == "" && param.Default == nil { - return fmt.Errorf("%s (%s): required environment variable %s is not set", path, param.Name, envName) - } - } - } - return nil -} - -func (v *SchemaValidator) validatePreconditions() error { - for i, precond := range v.config.Spec.Preconditions { - path := fmt.Sprintf("%s.%s[%d]", FieldSpec, FieldPreconditions, i) - - if precond.Name == "" { - return fmt.Errorf("%s.%s is required", path, FieldName) - } - - if !v.hasPreconditionLogic(precond) { - return fmt.Errorf("%s (%s): must specify %s, %s, or %s", - path, precond.Name, FieldAPICall, FieldExpression, FieldConditions) - } - - if precond.APICall != nil { - if err := v.validateAPICall(precond.APICall, path+"."+FieldAPICall); err != nil { - return err - } - } - } - return nil -} - -func (v *SchemaValidator) validateResources() error { - seen := make(map[string]bool) - - for i, resource := range v.config.Spec.Resources { - path := fmt.Sprintf("%s.%s[%d]", FieldSpec, FieldResources, i) - - if resource.Name == "" { - return fmt.Errorf("%s.%s is required", path, FieldName) - } - - // Validate resource name format for CEL compatibility - // Allows snake_case and camelCase, but NOT kebab-case (hyphens conflict with CEL minus operator) - if !validResourceNameRegex.MatchString(resource.Name) { - return fmt.Errorf("%s.%s %q: must start with lowercase letter and contain only letters, numbers, underscores (no hyphens)", path, FieldName, resource.Name) - } - - // Check for duplicate resource names - if seen[resource.Name] { - return fmt.Errorf("%s.%s %q: duplicate resource name", path, FieldName, resource.Name) - } - seen[resource.Name] = true - - if resource.Manifest == nil { - return fmt.Errorf("%s (%s): %s is required", path, resource.Name, FieldManifest) - } - - // Discovery is required for ALL resources to find them on subsequent messages - if err := v.validateResourceDiscovery(&resource, path); err != nil { - return err - } - } - return nil -} - -func (v *SchemaValidator) validateResourceDiscovery(resource *Resource, path string) error { - if resource.Discovery == nil { - return fmt.Errorf("%s (%s): %s is required to find the resource on subsequent messages", path, resource.Name, FieldDiscovery) - } - - // Namespace is optional - empty or "*" means all namespaces - - // Either byName or bySelectors must be configured - hasByName := resource.Discovery.ByName != "" - hasBySelectors := resource.Discovery.BySelectors != nil - - if !hasByName && !hasBySelectors { - return fmt.Errorf("%s (%s): %s must have either %s or %s configured", path, resource.Name, FieldDiscovery, FieldByName, FieldBySelectors) - } - - // If bySelectors is used, at least one selector must be defined - if hasBySelectors { - if err := v.validateSelectors(resource.Discovery.BySelectors, path, resource.Name); err != nil { - return err - } - } - - return nil -} - -func (v *SchemaValidator) validateSelectors(selectors *SelectorConfig, path, resourceName string) error { - if selectors == nil { - return fmt.Errorf("%s (%s): %s is nil", path, resourceName, FieldBySelectors) - } - - if len(selectors.LabelSelector) == 0 { - return fmt.Errorf("%s (%s): %s must have %s defined", path, resourceName, FieldBySelectors, FieldLabelSelector) - } - - return nil -} - -func (v *SchemaValidator) validatePostActions() error { - if v.config.Spec.Post == nil { - return nil - } - - for i, action := range v.config.Spec.Post.PostActions { - path := fmt.Sprintf("%s.%s.%s[%d]", FieldSpec, FieldPost, FieldPostActions, i) - - if action.Name == "" { - return fmt.Errorf("%s.%s is required", path, FieldName) - } - - if action.APICall != nil { - if err := v.validateAPICall(action.APICall, path+"."+FieldAPICall); err != nil { - return err - } - } - } - return nil -} - -func (v *SchemaValidator) validatePayloads() error { - if v.config.Spec.Post == nil { - return nil - } - - for i, payload := range v.config.Spec.Post.Payloads { - path := fmt.Sprintf("%s.%s.%s[%d]", FieldSpec, FieldPost, FieldPayloads, i) - - if payload.Name == "" { - return fmt.Errorf("%s.%s is required", path, FieldName) - } - - if err := payload.Validate(); err != nil { - return fmt.Errorf("%s (%s): %w", path, payload.Name, err) - } - } - return nil -} - -func (v *SchemaValidator) validateAPICall(apiCall *APICall, path string) error { - if apiCall == nil { - return fmt.Errorf("%s: %s is nil", path, FieldAPICall) - } - - if apiCall.Method == "" { - return fmt.Errorf("%s.%s is required", path, FieldMethod) - } - - if !slices.Contains(ValidHTTPMethods, apiCall.Method) { - return fmt.Errorf("%s.%s %q is invalid (allowed: %s)", path, FieldMethod, apiCall.Method, strings.Join(ValidHTTPMethods, ", ")) - } - - if apiCall.URL == "" { - return fmt.Errorf("%s.%s is required", path, FieldURL) - } - - return nil -} - -// ----------------------------------------------------------------------------- -// File Reference Validation -// ----------------------------------------------------------------------------- - -func (v *SchemaValidator) validateFileReferences() error { - var errors []string - - // Validate buildRef in spec.post.payloads - if v.config.Spec.Post != nil { - for i, payload := range v.config.Spec.Post.Payloads { - if payload.BuildRef != "" { - path := fmt.Sprintf("%s.%s.%s[%d].%s", FieldSpec, FieldPost, FieldPayloads, i, FieldBuildRef) - if err := v.validateFileExists(payload.BuildRef, path); err != nil { - errors = append(errors, err.Error()) - } - } - } - } - - // Validate manifest.ref and manifest.refs in spec.resources - for i, resource := range v.config.Spec.Resources { - refs := resource.GetManifestRefs() - for j, ref := range refs { - var path string - if len(refs) == 1 { - path = fmt.Sprintf("%s.%s[%d].%s.%s", FieldSpec, FieldResources, i, FieldManifest, FieldRef) - } else { - path = fmt.Sprintf("%s.%s[%d].%s.%s[%d]", FieldSpec, FieldResources, i, FieldManifest, FieldRefs, j) - } - if err := v.validateFileExists(ref, path); err != nil { - errors = append(errors, err.Error()) - } - } - } - - if len(errors) > 0 { - return fmt.Errorf("file reference errors:\n - %s", strings.Join(errors, "\n - ")) - } - return nil -} - -func (v *SchemaValidator) validateFileExists(refPath, configPath string) error { - if refPath == "" { - return fmt.Errorf("%s: file reference is empty", configPath) - } - - fullPath, err := v.resolvePath(refPath) - if err != nil { - return fmt.Errorf("%s: %w", configPath, err) - } - - // Check if file exists - info, err := os.Stat(fullPath) - if err != nil { - if os.IsNotExist(err) { - return fmt.Errorf("%s: referenced file %q does not exist (resolved to %q)", configPath, refPath, fullPath) - } - return fmt.Errorf("%s: error checking file %q: %w", configPath, refPath, err) - } - - // Ensure it's a file, not a directory - if info.IsDir() { - return fmt.Errorf("%s: referenced path %q is a directory, not a file", configPath, refPath) - } - - return nil -} - -// ----------------------------------------------------------------------------- -// File Reference Loading -// ----------------------------------------------------------------------------- - -func (v *SchemaValidator) loadFileReferences() error { - // Load manifest.ref or manifest.refs in spec.resources - for i := range v.config.Spec.Resources { - resource := &v.config.Spec.Resources[i] - refs := resource.GetManifestRefs() - if len(refs) == 0 { - continue - } - - // Load all referenced files - items := make([]map[string]interface{}, 0, len(refs)) - for j, ref := range refs { - content, err := v.loadYAMLFile(ref) - if err != nil { - if len(refs) == 1 { - return fmt.Errorf("%s.%s[%d].%s.%s: %w", FieldSpec, FieldResources, i, FieldManifest, FieldRef, err) - } - return fmt.Errorf("%s.%s[%d].%s.%s[%d]: %w", FieldSpec, FieldResources, i, FieldManifest, FieldRefs, j, err) - } - items = append(items, content) - } - - // Store loaded items - if len(items) == 1 { - // Single ref: replace manifest with content (backward compatible) - resource.Manifest = items[0] - } else { - // Multiple refs: store in ManifestItems array - resource.ManifestItems = items - } - } - - // Load buildRef in spec.post.payloads - if v.config.Spec.Post != nil { - for i := range v.config.Spec.Post.Payloads { - payload := &v.config.Spec.Post.Payloads[i] - if payload.BuildRef != "" { - content, err := v.loadYAMLFile(payload.BuildRef) - if err != nil { - return fmt.Errorf("%s.%s.%s[%d].%s: %w", FieldSpec, FieldPost, FieldPayloads, i, FieldBuildRef, err) - } - payload.BuildRefContent = content - } - } - } - - return nil -} - -func (v *SchemaValidator) loadYAMLFile(refPath string) (map[string]interface{}, error) { - fullPath, err := v.resolvePath(refPath) - if err != nil { - return nil, err - } - - data, err := os.ReadFile(fullPath) - if err != nil { - return nil, fmt.Errorf("failed to read file %q: %w", fullPath, err) - } - - var content map[string]interface{} - if err := yaml.Unmarshal(data, &content); err != nil { - return nil, fmt.Errorf("failed to parse YAML file %q: %w", fullPath, err) - } - - return content, nil -} - -// resolvePath resolves a relative path against the base directory and validates -// that the resolved path does not escape the base directory. -// Returns the resolved path and an error if path traversal is detected. -func (v *SchemaValidator) resolvePath(refPath string) (string, error) { - // Get absolute, clean path for base directory - baseAbs, err := filepath.Abs(v.baseDir) - if err != nil { - return "", fmt.Errorf("failed to resolve base directory: %w", err) - } - baseClean := filepath.Clean(baseAbs) - - var targetPath string - if filepath.IsAbs(refPath) { - targetPath = filepath.Clean(refPath) - } else { - targetPath = filepath.Clean(filepath.Join(baseClean, refPath)) - } - - // Check if target path is within base directory using filepath.Rel - rel, err := filepath.Rel(baseClean, targetPath) - if err != nil { - return "", fmt.Errorf("path %q escapes base directory", refPath) - } - - // If the relative path starts with "..", it escapes the base directory - if strings.HasPrefix(rel, "..") { - return "", fmt.Errorf("path %q escapes base directory", refPath) - } - - return targetPath, nil -} - -// ----------------------------------------------------------------------------- -// Validation Helpers -// ----------------------------------------------------------------------------- - -func (v *SchemaValidator) hasPreconditionLogic(precond Precondition) bool { - return precond.APICall != nil || - precond.Expression != "" || - len(precond.Conditions) > 0 -} - -// ----------------------------------------------------------------------------- -// Package-level Helper Functions (for backward compatibility) -// ----------------------------------------------------------------------------- - -// IsSupportedAPIVersion checks if the given apiVersion is supported -func IsSupportedAPIVersion(apiVersion string) bool { - for _, v := range SupportedAPIVersions { - if v == apiVersion { - return true - } - } - return false -} - -// ValidateAdapterVersion validates the config's adapter version matches the expected version -func ValidateAdapterVersion(config *AdapterConfig, expectedVersion string) error { - if expectedVersion == "" { - return nil - } - - configVersion := config.Spec.Adapter.Version - if configVersion != expectedVersion { - return fmt.Errorf("adapter version mismatch: config %q != adapter %q", - configVersion, expectedVersion) - } - - return nil -} - -// ----------------------------------------------------------------------------- -// Legacy Functions (for backward compatibility with loader.go) -// ----------------------------------------------------------------------------- - -func validateAPIVersionAndKind(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validateAPIVersionAndKind() -} - -func validateMetadata(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validateMetadata() -} - -func validateAdapterSpec(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validateAdapterSpec() -} - -func validateParams(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validateParams() -} - -func validatePreconditions(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validatePreconditions() -} - -func validateResources(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validateResources() -} - -func validatePostActions(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validatePostActions() -} - -func validatePayloads(config *AdapterConfig) error { - return NewSchemaValidator(config, "").validatePayloads() -} - -func validateFileReferences(config *AdapterConfig, baseDir string) error { - return NewSchemaValidator(config, baseDir).ValidateFileReferences() -} - -func loadFileReferences(config *AdapterConfig, baseDir string) error { - return NewSchemaValidator(config, baseDir).LoadFileReferences() -} diff --git a/internal/config_loader/validator_test.go b/internal/config_loader/validator_test.go index 4cb4773..7ccd1cf 100644 --- a/internal/config_loader/validator_test.go +++ b/internal/config_loader/validator_test.go @@ -28,7 +28,7 @@ func TestValidateConditionOperators(t *testing.T) { withCondition := func(cond Condition) *AdapterConfig { cfg := baseConfig() cfg.Spec.Preconditions = []Precondition{{ - Name: "checkStatus", + ActionBase: ActionBase{Name: "checkStatus"}, Conditions: []Condition{cond}, }} return cfg @@ -37,7 +37,7 @@ func TestValidateConditionOperators(t *testing.T) { t.Run("valid operators", func(t *testing.T) { cfg := baseConfig() cfg.Spec.Preconditions = []Precondition{{ - Name: "checkStatus", + ActionBase: ActionBase{Name: "checkStatus"}, Conditions: []Condition{ {Field: "status", Operator: "equals", Value: "Ready"}, {Field: "provider", Operator: "in", Value: []interface{}{"aws", "gcp"}}, @@ -49,16 +49,16 @@ func TestValidateConditionOperators(t *testing.T) { t.Run("invalid operator", func(t *testing.T) { cfg := withCondition(Condition{Field: "status", Operator: "invalidOp", Value: "Ready"}) - err := newValidator(cfg).Validate() + err := newValidator(cfg).ValidateStructure() require.Error(t, err) assert.Contains(t, err.Error(), "invalid operator") }) t.Run("missing operator", func(t *testing.T) { cfg := withCondition(Condition{Field: "status", Value: "Ready"}) - err := newValidator(cfg).Validate() + err := newValidator(cfg).ValidateStructure() require.Error(t, err) - assert.Contains(t, err.Error(), "operator is required") + assert.Contains(t, err.Error(), "operator") }) t.Run("missing value for equals operator", func(t *testing.T) { @@ -94,6 +94,20 @@ func TestValidateConditionOperators(t *testing.T) { assert.NoError(t, newValidator(cfg).Validate()) }) + t.Run("exists operator with value should fail", func(t *testing.T) { + cfg := withCondition(Condition{Field: "vpcId", Operator: "exists", Value: "any-value"}) + err := newValidator(cfg).Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "value/values should not be set for operator \"exists\"") + }) + + t.Run("exists operator with list value should fail", func(t *testing.T) { + cfg := withCondition(Condition{Field: "vpcId", Operator: "exists", Value: []interface{}{"a", "b"}}) + err := newValidator(cfg).Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "value/values should not be set for operator \"exists\"") + }) + t.Run("missing value for greaterThan operator", func(t *testing.T) { cfg := withCondition(Condition{Field: "count", Operator: "greaterThan"}) err := newValidator(cfg).Validate() @@ -110,8 +124,10 @@ func TestValidateTemplateVariables(t *testing.T) { {Name: "apiUrl", Source: "env.API_URL"}, } cfg.Spec.Preconditions = []Precondition{{ - Name: "checkCluster", - APICall: &APICall{Method: "GET", URL: "{{ .apiUrl }}/clusters/{{ .clusterId }}"}, + ActionBase: ActionBase{ + Name: "checkCluster", + APICall: &APICall{Method: "GET", URL: "{{ .apiUrl }}/clusters/{{ .clusterId }}"}, + }, }} assert.NoError(t, newValidator(cfg).Validate()) }) @@ -120,8 +136,10 @@ func TestValidateTemplateVariables(t *testing.T) { cfg := baseConfig() cfg.Spec.Params = []Parameter{{Name: "clusterId", Source: "event.cluster_id"}} cfg.Spec.Preconditions = []Precondition{{ - Name: "checkCluster", - APICall: &APICall{Method: "GET", URL: "{{ .undefinedVar }}/clusters/{{ .clusterId }}"}, + ActionBase: ActionBase{ + Name: "checkCluster", + APICall: &APICall{Method: "GET", URL: "{{ .undefinedVar }}/clusters/{{ .clusterId }}"}, + }, }} err := newValidator(cfg).Validate() require.Error(t, err) @@ -149,9 +167,11 @@ func TestValidateTemplateVariables(t *testing.T) { cfg := baseConfig() cfg.Spec.Params = []Parameter{{Name: "apiUrl", Source: "env.API_URL"}} cfg.Spec.Preconditions = []Precondition{{ - Name: "getCluster", - APICall: &APICall{Method: "GET", URL: "{{ .apiUrl }}/clusters"}, - Capture: []CaptureField{{Name: "clusterName", Field: "metadata.name"}}, + ActionBase: ActionBase{ + Name: "getCluster", + APICall: &APICall{Method: "GET", URL: "{{ .apiUrl }}/clusters"}, + }, + Capture: []CaptureField{{Name: "clusterName", FieldExpressionDef: FieldExpressionDef{Field: "metadata.name"}}}, }} cfg.Spec.Resources = []Resource{{ Name: "testNs", @@ -170,7 +190,7 @@ func TestValidateCELExpressions(t *testing.T) { // Helper to create config with a CEL expression precondition withExpression := func(expr string) *AdapterConfig { cfg := baseConfig() - cfg.Spec.Preconditions = []Precondition{{Name: "check", Expression: expr}} + cfg.Spec.Preconditions = []Precondition{{ActionBase: ActionBase{Name: "check"}, Expression: expr}} return cfg } @@ -376,8 +396,8 @@ func TestValidate(t *testing.T) { // Test that Validate catches multiple errors cfg := baseConfig() cfg.Spec.Preconditions = []Precondition{ - {Name: "check1", Conditions: []Condition{{Field: "status", Operator: "badOperator", Value: "Ready"}}}, - {Name: "check2", Expression: "invalid ))) syntax"}, + {ActionBase: ActionBase{Name: "check1"}, Conditions: []Condition{{Field: "status", Operator: "badOperator", Value: "Ready"}}}, + {ActionBase: ActionBase{Name: "check2"}, Expression: "invalid ))) syntax"}, } cfg.Spec.Resources = []Resource{{ Name: "testNs", @@ -442,7 +462,7 @@ func TestPayloadValidate(t *testing.T) { BuildRef: "templates/payload.yaml", }, wantError: true, - errorMsg: "build and buildRef are mutually exclusive", + errorMsg: "mutually exclusive", }, { name: "invalid - neither Build nor BuildRef set", @@ -450,18 +470,21 @@ func TestPayloadValidate(t *testing.T) { Name: "test", }, wantError: true, - errorMsg: "either build or buildRef must be set", + errorMsg: "must have either", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.payload.Validate() + errs := ValidateStruct(&tt.payload) if tt.wantError { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.errorMsg) + require.NotNil(t, errs) + require.True(t, errs.HasErrors()) + assert.Contains(t, errs.Error(), tt.errorMsg) } else { - assert.NoError(t, err) + if errs != nil { + assert.False(t, errs.HasErrors(), "unexpected error: %v", errs) + } } }) } @@ -502,13 +525,13 @@ spec: status: "ready" buildRef: "templates/payload.yaml"`) require.Error(t, err) - assert.Contains(t, err.Error(), "build and buildRef are mutually exclusive") + assert.Contains(t, err.Error(), "mutually exclusive") }) t.Run("invalid - neither build nor buildRef specified", func(t *testing.T) { _, err := parseWithPayloads(` - name: "statusPayload"`) require.Error(t, err) - assert.Contains(t, err.Error(), "either build or buildRef must be set") + assert.Contains(t, err.Error(), "must have either") }) t.Run("invalid - payload name missing", func(t *testing.T) { @@ -527,7 +550,7 @@ spec: data: "test" buildRef: "templates/conflict.yaml"`) require.Error(t, err) - assert.Contains(t, err.Error(), "payload2") + assert.Contains(t, err.Error(), "payloads[1]") }) } @@ -536,8 +559,10 @@ func TestValidateCaptureFields(t *testing.T) { withCapture := func(captures []CaptureField) *AdapterConfig { cfg := baseConfig() cfg.Spec.Preconditions = []Precondition{{ - Name: "getStatus", - APICall: &APICall{Method: "GET", URL: "http://example.com/api"}, + ActionBase: ActionBase{ + Name: "getStatus", + APICall: &APICall{Method: "GET", URL: "http://example.com/api"}, + }, Capture: captures, }} return cfg @@ -545,35 +570,79 @@ func TestValidateCaptureFields(t *testing.T) { t.Run("valid capture with field only", func(t *testing.T) { cfg := withCapture([]CaptureField{ - {Name: "clusterName", Field: "metadata.name"}, - {Name: "clusterPhase", Field: "status.phase"}, + {Name: "clusterName", FieldExpressionDef: FieldExpressionDef{Field: "metadata.name"}}, + {Name: "clusterPhase", FieldExpressionDef: FieldExpressionDef{Field: "status.phase"}}, }) assert.NoError(t, newValidator(cfg).Validate()) }) t.Run("valid capture with expression only", func(t *testing.T) { - cfg := withCapture([]CaptureField{{Name: "activeCount", Expression: "1 + 1"}}) + cfg := withCapture([]CaptureField{{Name: "activeCount", FieldExpressionDef: FieldExpressionDef{Expression: "1 + 1"}}}) assert.NoError(t, newValidator(cfg).Validate()) }) t.Run("invalid - both field and expression set", func(t *testing.T) { - cfg := withCapture([]CaptureField{{Name: "conflicting", Field: "metadata.name", Expression: "1 + 1"}}) - err := newValidator(cfg).Validate() + cfg := withCapture([]CaptureField{{Name: "conflicting", FieldExpressionDef: FieldExpressionDef{Field: "metadata.name", Expression: "1 + 1"}}}) + err := newValidator(cfg).ValidateStructure() require.Error(t, err) - assert.Contains(t, err.Error(), "cannot have both 'field' and 'expression' set") + assert.Contains(t, err.Error(), "mutually exclusive") }) t.Run("invalid - neither field nor expression set", func(t *testing.T) { cfg := withCapture([]CaptureField{{Name: "empty"}}) - err := newValidator(cfg).Validate() + err := newValidator(cfg).ValidateStructure() require.Error(t, err) - assert.Contains(t, err.Error(), "must have either 'field' or 'expression' set") + assert.Contains(t, err.Error(), "must have either") }) t.Run("invalid - capture name missing", func(t *testing.T) { - cfg := withCapture([]CaptureField{{Field: "metadata.name"}}) - err := newValidator(cfg).Validate() + cfg := withCapture([]CaptureField{{FieldExpressionDef: FieldExpressionDef{Field: "metadata.name"}}}) + err := newValidator(cfg).ValidateStructure() require.Error(t, err) - assert.Contains(t, err.Error(), "capture name is required") + assert.Contains(t, err.Error(), "name is required") }) } + +func TestYamlFieldName(t *testing.T) { + // Ensure validator is initialized (populates fieldNameCache) + getStructValidator() + + tests := []struct { + goFieldName string + expectedYaml string + }{ + {"ByName", "byName"}, + {"BySelectors", "bySelectors"}, + {"Field", "field"}, + {"Expression", "expression"}, + {"APIVersion", "apiVersion"}, + {"Name", "name"}, + {"Namespace", "namespace"}, + {"LabelSelector", "labelSelector"}, + } + + for _, tt := range tests { + t.Run(tt.goFieldName, func(t *testing.T) { + result := yamlFieldName(tt.goFieldName) + assert.Equal(t, tt.expectedYaml, result) + }) + } +} + +func TestFieldNameCachePopulated(t *testing.T) { + // Ensure validator is initialized + getStructValidator() + + // Verify key fields are in the cache + expectedFields := []string{ + "ByName", "BySelectors", "Field", "Expression", + "Name", "Namespace", "APIVersion", "Kind", + } + + for _, field := range expectedFields { + t.Run(field, func(t *testing.T) { + _, ok := fieldNameCache[field] + assert.True(t, ok, "field %s should be in cache", field) + }) + } +} diff --git a/internal/criteria/README.md b/internal/criteria/README.md index 0c71980..0fcb01e 100644 --- a/internal/criteria/README.md +++ b/internal/criteria/README.md @@ -42,7 +42,7 @@ ctx.Set("provider", "aws") ctx.Set("nodeCount", 5) // Create evaluator -evaluator := criteria.NewEvaluator(ctx, log) +evaluator, _ := criteria.NewEvaluator(context.Background(), ctx, log) // Evaluate a single condition result, err := evaluator.EvaluateCondition( @@ -141,34 +141,17 @@ See: [Kubernetes JSONPath Reference](https://kubernetes.io/docs/reference/kubect The `ExtractValue` method provides a unified interface for extracting values using either field (JSONPath) or expression (CEL). This is used by captures, conditions, and payload building. ```go -// Create evaluator with your data context -ctx := criteria.NewEvaluationContext() -ctx.SetVariablesFromMap(responseData) -evaluator, _ := criteria.NewEvaluator(context.Background(), ctx, log) - -// Extract using JSONPath +// Extract using JSONPath (using evaluator from Basic Evaluation example above) result, err := evaluator.ExtractValue("status.phase", "") -if err != nil { - // Parse error only - field not found returns nil value, not error - log.Fatal(err) -} -if result.Value != nil { - fmt.Println("Phase:", result.Value) -} else { - fmt.Println("Field not found, using default") -} // Extract using CEL expression result, err = evaluator.ExtractValue("", "items.filter(i, i.status == 'active').size()") -if err != nil { - log.Fatal(err) -} -fmt.Println("Active count:", result.Value) ``` The `ExtractValueResult` contains: - `Value`: The extracted value (nil if field not found or empty) - `Source`: The field path or expression used +- `Error`: Runtime extraction error (if any) **Error handling:** - Returns `error` (2nd return) only for **parse errors** (invalid JSONPath/CEL syntax) @@ -189,146 +172,34 @@ if ok { fmt.Println("Value:", val) } -// Get nested field -val, err := ctx.GetField("cluster.status.phase") - // Merge contexts ctx2 := criteria.NewEvaluationContext() ctx2.Set("newKey", "newValue") ctx.Merge(ctx2) // ctx now has both key and newKey ``` -## Examples - -### Example 1: Cluster Validation +#### GetField Method ```go -// Simulate cluster details from API -ctx := criteria.NewEvaluationContext() -ctx.Set("clusterPhase", "Ready") -ctx.Set("cloudProvider", "aws") -ctx.Set("vpcId", "vpc-12345") - -evaluator := criteria.NewEvaluator(ctx, log) - -// Validate cluster is in correct phase -phaseResult, _ := evaluator.EvaluateCondition( - "clusterPhase", - criteria.OperatorIn, - []interface{}{"Provisioning", "Installing", "Ready"}, -) - -// Validate provider is allowed -providerResult, _ := evaluator.EvaluateCondition( - "cloudProvider", - criteria.OperatorIn, - []interface{}{"aws", "gcp", "azure"}, -) - -// Validate VPC exists -vpcResult, _ := evaluator.EvaluateCondition( - "vpcId", - criteria.OperatorExists, - nil, -) - -if phaseResult.Matched && providerResult.Matched && vpcResult.Matched { - fmt.Println("Cluster validation passed") -} +func (c *EvaluationContext) GetField(path string) (*FieldResult, error) ``` -### Example 2: Resource Status Check +Retrieves a field using dot notation (e.g., `"cluster.status.phase"`) or JSONPath (e.g., `"{.items[0].name}"`). -```go -// Simulate resource status -ctx := criteria.NewEvaluationContext() -ctx.Set("resources", map[string]interface{}{ - "clusterNamespace": map[string]interface{}{ - "status": map[string]interface{}{ - "phase": "Active", - }, - }, - "clusterController": map[string]interface{}{ - "status": map[string]interface{}{ - "replicas": 3, - "readyReplicas": 3, - }, - }, -}) - -evaluator := criteria.NewEvaluator(ctx, log) - -// Check namespace is active -nsResult, _ := evaluator.EvaluateCondition( - "resources.clusterNamespace.status.phase", - criteria.OperatorEquals, - "Active", -) - -// Check all replicas are ready -replicasResult, _ := evaluator.EvaluateCondition( - "resources.clusterController.status.readyReplicas", - criteria.OperatorGreaterThan, - 0, -) - -if nsResult.Matched && replicasResult.Matched { - fmt.Println("Resources are healthy") -} -``` - -### Example 3: Array and String Contains - -```go -ctx := criteria.NewEvaluationContext() -evaluator := criteria.NewEvaluator(ctx, log) - -// String contains -ctx.Set("message", "Deployment ready and healthy") -result, _ := evaluator.EvaluateCondition( - "message", - criteria.OperatorContains, - "ready", -) -fmt.Println("Message contains 'ready':", result.Matched) // true - -// Array contains -ctx.Set("tags", []interface{}{"production", "us-east-1", "critical"}) -result, _ = evaluator.EvaluateCondition( - "tags", - criteria.OperatorContains, - "production", -) -fmt.Println("Tags contain 'production':", result.Matched) // true -``` - -### Example 4: Numeric Comparisons - -```go -ctx := criteria.NewEvaluationContext() -ctx.Set("nodeCount", 5) -ctx.Set("minNodes", 1) -ctx.Set("maxNodes", 10) +**Return Values:** -evaluator := criteria.NewEvaluator(ctx, log) +- `*FieldResult.Value`: The extracted value (`string`, `float64`, `bool`, `map[string]interface{}`, `[]interface{}`, or `nil` if not found) +- `*FieldResult.Error`: Runtime extraction error (e.g., JSONPath execution failure) +- `error` (2nd return): Parse error for invalid path syntax -// Check if within range -aboveMinResult, _ := evaluator.EvaluateCondition( - "nodeCount", - criteria.OperatorGreaterThan, - 0, // nodeCount > 0 means >= 1 -) - -belowMaxResult, _ := evaluator.EvaluateCondition( - "nodeCount", - criteria.OperatorLessThan, - 11, // nodeCount < 11 means <= 10 -) +**Error Conditions:** -if aboveMinResult.Matched && belowMaxResult.Matched { - fmt.Println("Node count is within valid range") -} -``` +| Condition | `error` (2nd return) | `result.Value` | `result.Error` | +|-----------|---------------------|----------------|----------------| +| Empty path | `"empty field path"` | - | - | +| Invalid JSONPath syntax | `"invalid field path..."` | - | - | +| Field not found | `nil` | `nil` | `nil` | +| JSONPath execution failure | `nil` | `nil` | set | ## Integration with Config Loader @@ -353,7 +224,7 @@ ctx.Set("cloudProvider", "aws") ctx.Set("vpcId", "vpc-12345") // Evaluate precondition conditions -evaluator := criteria.NewEvaluator(ctx, log) +evaluator, _ := criteria.NewEvaluator(context.Background(), ctx, log) conditions := make([]criteria.ConditionDef, len(precond.Conditions)) for i, cond := range precond.Conditions { conditions[i] = criteria.ConditionDef{ @@ -383,7 +254,7 @@ The package provides descriptive error messages: ctx := criteria.NewEvaluationContext() ctx.Set("count", "not a number") -evaluator := criteria.NewEvaluator(ctx, log) +evaluator, _ := criteria.NewEvaluator(context.Background(), ctx, log) result, err := evaluator.EvaluateCondition( "count", criteria.OperatorGreaterThan, diff --git a/internal/executor/README.md b/internal/executor/README.md index c90aed7..85224ef 100644 --- a/internal/executor/README.md +++ b/internal/executor/README.md @@ -141,10 +141,28 @@ params: - name: "apiToken" source: "env.API_TOKEN" required: true + - name: "nodeCount" + source: "event.spec.nodes" + type: "int" # Convert to int64 + - name: "enableFeature" + source: "env.ENABLE_FEATURE" + type: "bool" # Convert to bool + default: false ``` +#### Supported Parameter Types + +| Type | Description | Conversion Notes | +|------|-------------|-----------------| +| `string` | String value (default) | Any value converted to string | +| `int`, `int64` | Integer value | Strings parsed, floats truncated | +| `float`, `float64` | Floating point value | Strings parsed | +| `bool` | Boolean value | Supports: `true/false`, `yes/no`, `on/off`, `1/0` | + +If `type` is not specified, the value retains its original type from the source. + ### Phase 2: Precondition Evaluation Executes preconditions with optional API calls and condition evaluation: diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index c9c3f14..9f3cc0a 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -450,9 +450,9 @@ func TestSequentialExecution_Preconditions(t *testing.T) { { name: "all pass - all executed", preconditions: []config_loader.Precondition{ - {Name: "precond1", Expression: "true"}, - {Name: "precond2", Expression: "true"}, - {Name: "precond3", Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond1"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond2"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond3"}, Expression: "true"}, }, expectedResults: 3, expectError: false, @@ -462,9 +462,9 @@ func TestSequentialExecution_Preconditions(t *testing.T) { { name: "first fails - stops immediately", preconditions: []config_loader.Precondition{ - {Name: "precond1", Expression: "false"}, - {Name: "precond2", Expression: "true"}, - {Name: "precond3", Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond1"}, Expression: "false"}, + {ActionBase: config_loader.ActionBase{Name: "precond2"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond3"}, Expression: "true"}, }, expectedResults: 1, expectError: false, @@ -474,9 +474,9 @@ func TestSequentialExecution_Preconditions(t *testing.T) { { name: "second fails - first executes, stops at second", preconditions: []config_loader.Precondition{ - {Name: "precond1", Expression: "true"}, - {Name: "precond2", Expression: "false"}, - {Name: "precond3", Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond1"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond2"}, Expression: "false"}, + {ActionBase: config_loader.ActionBase{Name: "precond3"}, Expression: "true"}, }, expectedResults: 2, expectError: false, @@ -486,9 +486,9 @@ func TestSequentialExecution_Preconditions(t *testing.T) { { name: "third fails - first two execute, stops at third", preconditions: []config_loader.Precondition{ - {Name: "precond1", Expression: "true"}, - {Name: "precond2", Expression: "true"}, - {Name: "precond3", Expression: "false"}, + {ActionBase: config_loader.ActionBase{Name: "precond1"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond2"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "precond3"}, Expression: "false"}, }, expectedResults: 3, expectError: false, @@ -657,9 +657,9 @@ func TestSequentialExecution_PostActions(t *testing.T) { { name: "all log actions succeed", postActions: []config_loader.PostAction{ - {Name: "log1", Log: &config_loader.LogAction{Message: "msg1"}}, - {Name: "log2", Log: &config_loader.LogAction{Message: "msg2"}}, - {Name: "log3", Log: &config_loader.LogAction{Message: "msg3"}}, + {ActionBase: config_loader.ActionBase{Name: "log1", Log: &config_loader.LogAction{Message: "msg1"}}}, + {ActionBase: config_loader.ActionBase{Name: "log2", Log: &config_loader.LogAction{Message: "msg2"}}}, + {ActionBase: config_loader.ActionBase{Name: "log3", Log: &config_loader.LogAction{Message: "msg3"}}}, }, expectedResults: 3, expectError: false, @@ -727,9 +727,9 @@ func TestSequentialExecution_SkipReasonCapture(t *testing.T) { { name: "first precondition not met", preconditions: []config_loader.Precondition{ - {Name: "check1", Expression: "false"}, - {Name: "check2", Expression: "true"}, - {Name: "check3", Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "check1"}, Expression: "false"}, + {ActionBase: config_loader.ActionBase{Name: "check2"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "check3"}, Expression: "true"}, }, expectedStatus: StatusSuccess, // Successful execution, just resources skipped expectSkipped: true, @@ -737,9 +737,9 @@ func TestSequentialExecution_SkipReasonCapture(t *testing.T) { { name: "second precondition not met", preconditions: []config_loader.Precondition{ - {Name: "check1", Expression: "true"}, - {Name: "check2", Expression: "false"}, - {Name: "check3", Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "check1"}, Expression: "true"}, + {ActionBase: config_loader.ActionBase{Name: "check2"}, Expression: "false"}, + {ActionBase: config_loader.ActionBase{Name: "check3"}, Expression: "true"}, }, expectedStatus: StatusSuccess, // Successful execution, just resources skipped expectSkipped: true, diff --git a/internal/executor/param_extractor.go b/internal/executor/param_extractor.go index f70d701..2f7eb8e 100644 --- a/internal/executor/param_extractor.go +++ b/internal/executor/param_extractor.go @@ -3,7 +3,9 @@ package executor import ( "context" "fmt" + "math" "os" + "strconv" "strings" "github.com/openshift-hyperfleet/hyperfleet-adapter/internal/config_loader" @@ -27,11 +29,32 @@ func extractConfigParams(config *config_loader.AdapterConfig, execCtx *Execution continue } - // Apply default if value is nil or empty string - if (value == nil || value == "") && param.Default != nil { + // Apply default if value is nil or (for strings) empty + isEmpty := value == nil + if s, ok := value.(string); ok && s == "" { + isEmpty = true + } + if isEmpty && param.Default != nil { value = param.Default } + // Apply type conversion if specified + if value != nil && param.Type != "" { + converted, convErr := convertParamType(value, param.Type) + if convErr != nil { + if param.Required { + return NewExecutorError(PhaseParamExtraction, param.Name, + fmt.Sprintf("failed to convert parameter '%s' to type '%s'", param.Name, param.Type), convErr) + } + // Use default for non-required params if conversion fails + if param.Default != nil { + execCtx.Params[param.Name] = param.Default + } + continue + } + value = converted + } + if value != nil { execCtx.Params[param.Name] = value } @@ -138,3 +161,170 @@ func addMetadataParams(config *config_loader.AdapterConfig, execCtx *ExecutionCo "labels": config.Metadata.Labels, } } + +// convertParamType converts a value to the specified type +// Supported types: string, int, int64, float, float64, bool +func convertParamType(value interface{}, targetType string) (interface{}, error) { + // If value is already the target type, return as-is + switch targetType { + case "string": + return convertToString(value) + case "int", "int64": + return convertToInt64(value) + case "float", "float64": + return convertToFloat64(value) + case "bool": + return convertToBool(value) + default: + return nil, fmt.Errorf("unsupported type: %s (supported: string, int, int64, float, float64, bool)", targetType) + } +} + +// convertToString converts a value to string +func convertToString(value interface{}) (string, error) { + switch v := value.(type) { + case string: + return v, nil + case int, int8, int16, int32, int64: + return fmt.Sprintf("%d", v), nil + case uint, uint8, uint16, uint32, uint64: + return fmt.Sprintf("%d", v), nil + case float32: + return strconv.FormatFloat(float64(v), 'f', -1, 32), nil + case float64: + return strconv.FormatFloat(v, 'f', -1, 64), nil + case bool: + return strconv.FormatBool(v), nil + default: + return fmt.Sprintf("%v", v), nil + } +} + +// convertToInt64 converts a value to int64 +func convertToInt64(value interface{}) (int64, error) { + switch v := value.(type) { + case int: + return int64(v), nil + case uint64: + if v > math.MaxInt64 { + return 0, fmt.Errorf("uint64 value %d overflows int64", v) + } + return int64(v), nil + case int8: + return int64(v), nil + case int16: + return int64(v), nil + case int32: + return int64(v), nil + case int64: + return v, nil + case uint: + return int64(v), nil + case uint8: + return int64(v), nil + case uint16: + return int64(v), nil + case uint32: + return int64(v), nil + case float32: + return int64(v), nil + case float64: + return int64(v), nil + case string: + // Try parsing as int first + if i, err := strconv.ParseInt(v, 10, 64); err == nil { + return i, nil + } + // Try parsing as float and convert + if f, err := strconv.ParseFloat(v, 64); err == nil { + return int64(f), nil + } + return 0, fmt.Errorf("cannot convert string '%s' to int", v) + case bool: + if v { + return 1, nil + } + return 0, nil + default: + return 0, fmt.Errorf("cannot convert %T to int", value) + } +} + +// convertToFloat64 converts a value to float64 +func convertToFloat64(value interface{}) (float64, error) { + switch v := value.(type) { + case float32: + return float64(v), nil + case float64: + return v, nil + case int: + return float64(v), nil + case int8: + return float64(v), nil + case int16: + return float64(v), nil + case int32: + return float64(v), nil + case int64: + return float64(v), nil + case uint: + return float64(v), nil + case uint8: + return float64(v), nil + case uint16: + return float64(v), nil + case uint32: + return float64(v), nil + case uint64: + return float64(v), nil + case string: + f, err := strconv.ParseFloat(v, 64) + if err != nil { + return 0, fmt.Errorf("cannot convert string '%s' to float: %w", v, err) + } + return f, nil + case bool: + if v { + return 1.0, nil + } + return 0.0, nil + default: + return 0, fmt.Errorf("cannot convert %T to float", value) + } +} + +// convertToBool converts a value to bool +func convertToBool(value interface{}) (bool, error) { + switch v := value.(type) { + case bool: + return v, nil + case string: + // Empty string is treated as false + if v == "" { + return false, nil + } + b, err := strconv.ParseBool(v) + if err != nil { + // Handle common truthy/falsy strings + lower := strings.ToLower(v) + switch lower { + case "yes", "y", "on", "1": + return true, nil + case "no", "n", "off", "0": + return false, nil + } + return false, fmt.Errorf("cannot convert string '%s' to bool", v) + } + return b, nil + case int, int8, int16, int32, int64: + return v != 0, nil + case uint, uint8, uint16, uint32, uint64: + return v != 0, nil + case float32: + return v != 0, nil + case float64: + return v != 0, nil + default: + return false, fmt.Errorf("cannot convert %T to bool", value) + } +} diff --git a/internal/executor/param_extractor_test.go b/internal/executor/param_extractor_test.go new file mode 100644 index 0000000..c991e83 --- /dev/null +++ b/internal/executor/param_extractor_test.go @@ -0,0 +1,203 @@ +package executor + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertParamType(t *testing.T) { + tests := []struct { + name string + value interface{} + targetType string + want interface{} + wantErr bool + }{ + // String conversions + {name: "string to string", value: "hello", targetType: "string", want: "hello"}, + {name: "int to string", value: 42, targetType: "string", want: "42"}, + {name: "float to string", value: 3.14, targetType: "string", want: "3.14"}, + {name: "bool to string", value: true, targetType: "string", want: "true"}, + + // Int conversions + {name: "int to int", value: 42, targetType: "int", want: int64(42)}, + {name: "int64 to int64", value: int64(42), targetType: "int64", want: int64(42)}, + {name: "string to int", value: "42", targetType: "int", want: int64(42)}, + {name: "float to int", value: 3.9, targetType: "int", want: int64(3)}, + {name: "string float to int", value: "3.9", targetType: "int", want: int64(3)}, + {name: "bool true to int", value: true, targetType: "int", want: int64(1)}, + {name: "bool false to int", value: false, targetType: "int", want: int64(0)}, + {name: "invalid string to int", value: "abc", targetType: "int", wantErr: true}, + + // Float conversions + {name: "float to float", value: 3.14, targetType: "float", want: 3.14}, + {name: "float64 to float64", value: float64(3.14), targetType: "float64", want: 3.14}, + {name: "int to float", value: 42, targetType: "float", want: float64(42)}, + {name: "string to float", value: "3.14", targetType: "float", want: 3.14}, + {name: "bool true to float", value: true, targetType: "float", want: 1.0}, + {name: "invalid string to float", value: "abc", targetType: "float", wantErr: true}, + + // Bool conversions + {name: "bool to bool", value: true, targetType: "bool", want: true}, + {name: "string true to bool", value: "true", targetType: "bool", want: true}, + {name: "string false to bool", value: "false", targetType: "bool", want: false}, + {name: "string yes to bool", value: "yes", targetType: "bool", want: true}, + {name: "string no to bool", value: "no", targetType: "bool", want: false}, + {name: "string 1 to bool", value: "1", targetType: "bool", want: true}, + {name: "string 0 to bool", value: "0", targetType: "bool", want: false}, + {name: "int 1 to bool", value: 1, targetType: "bool", want: true}, + {name: "int 0 to bool", value: 0, targetType: "bool", want: false}, + {name: "float non-zero to bool", value: 3.14, targetType: "bool", want: true}, + {name: "invalid string to bool", value: "maybe", targetType: "bool", wantErr: true}, + + // Unsupported type + {name: "unsupported type", value: "test", targetType: "unknown", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := convertParamType(tt.value, tt.targetType) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestConvertToString(t *testing.T) { + tests := []struct { + name string + value interface{} + want string + }{ + {name: "string", value: "hello", want: "hello"}, + {name: "int", value: 42, want: "42"}, + {name: "int64", value: int64(123), want: "123"}, + {name: "float64", value: 3.14159, want: "3.14159"}, + {name: "bool true", value: true, want: "true"}, + {name: "bool false", value: false, want: "false"}, + {name: "uint", value: uint(100), want: "100"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := convertToString(tt.value) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestConvertToInt64(t *testing.T) { + tests := []struct { + name string + value interface{} + want int64 + wantErr bool + }{ + {name: "int", value: 42, want: 42}, + {name: "int64", value: int64(123), want: 123}, + {name: "float64", value: 3.9, want: 3}, + {name: "string int", value: "42", want: 42}, + {name: "string float", value: "3.9", want: 3}, + {name: "uint", value: uint(100), want: 100}, + {name: "bool true", value: true, want: 1}, + {name: "bool false", value: false, want: 0}, + {name: "invalid string", value: "abc", wantErr: true}, + {name: "invalid type", value: []string{"a"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := convertToInt64(tt.value) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestConvertToFloat64(t *testing.T) { + tests := []struct { + name string + value interface{} + want float64 + wantErr bool + }{ + {name: "float64", value: 3.14, want: 3.14}, + {name: "float32", value: float32(2.5), want: 2.5}, + {name: "int", value: 42, want: 42.0}, + {name: "int64", value: int64(123), want: 123.0}, + {name: "string", value: "3.14", want: 3.14}, + {name: "uint", value: uint(100), want: 100.0}, + {name: "bool true", value: true, want: 1.0}, + {name: "bool false", value: false, want: 0.0}, + {name: "invalid string", value: "abc", wantErr: true}, + {name: "invalid type", value: []string{"a"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := convertToFloat64(tt.value) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.InDelta(t, tt.want, got, 0.001) + } + }) + } +} + +func TestConvertToBool(t *testing.T) { + tests := []struct { + name string + value interface{} + want bool + wantErr bool + }{ + {name: "bool true", value: true, want: true}, + {name: "bool false", value: false, want: false}, + {name: "string true", value: "true", want: true}, + {name: "string false", value: "false", want: false}, + {name: "string True", value: "True", want: true}, + {name: "string FALSE", value: "FALSE", want: false}, + {name: "string yes", value: "yes", want: true}, + {name: "string no", value: "no", want: false}, + {name: "string y", value: "y", want: true}, + {name: "string n", value: "n", want: false}, + {name: "string on", value: "on", want: true}, + {name: "string off", value: "off", want: false}, + {name: "string 1", value: "1", want: true}, + {name: "string 0", value: "0", want: false}, + {name: "string empty", value: "", want: false}, + {name: "int 1", value: 1, want: true}, + {name: "int 0", value: 0, want: false}, + {name: "int non-zero", value: 42, want: true}, + {name: "float non-zero", value: 3.14, want: true}, + {name: "float zero", value: 0.0, want: false}, + {name: "invalid string", value: "maybe", wantErr: true}, + {name: "invalid type", value: []string{"a"}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := convertToBool(tt.value) + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + diff --git a/internal/executor/post_action_executor_test.go b/internal/executor/post_action_executor_test.go index 8bcdd47..872a666 100644 --- a/internal/executor/post_action_executor_test.go +++ b/internal/executor/post_action_executor_test.go @@ -327,8 +327,10 @@ func TestPostActionExecutor_ExecuteAll(t *testing.T) { postConfig: &config_loader.PostConfig{ PostActions: []config_loader.PostAction{ { - Name: "log-status", - Log: &config_loader.LogAction{Message: "Processing complete", Level: "info"}, + ActionBase: config_loader.ActionBase{ + Name: "log-status", + Log: &config_loader.LogAction{Message: "Processing complete", Level: "info"}, + }, }, }, }, @@ -339,9 +341,9 @@ func TestPostActionExecutor_ExecuteAll(t *testing.T) { name: "multiple log actions", postConfig: &config_loader.PostConfig{ PostActions: []config_loader.PostAction{ - {Name: "log1", Log: &config_loader.LogAction{Message: "Step 1", Level: "info"}}, - {Name: "log2", Log: &config_loader.LogAction{Message: "Step 2", Level: "info"}}, - {Name: "log3", Log: &config_loader.LogAction{Message: "Step 3", Level: "info"}}, + {ActionBase: config_loader.ActionBase{Name: "log1", Log: &config_loader.LogAction{Message: "Step 1", Level: "info"}}}, + {ActionBase: config_loader.ActionBase{Name: "log2", Log: &config_loader.LogAction{Message: "Step 2", Level: "info"}}}, + {ActionBase: config_loader.ActionBase{Name: "log3", Log: &config_loader.LogAction{Message: "Step 3", Level: "info"}}}, }, }, expectedResults: 3, @@ -359,7 +361,7 @@ func TestPostActionExecutor_ExecuteAll(t *testing.T) { }, }, PostActions: []config_loader.PostAction{ - {Name: "log1", Log: &config_loader.LogAction{Message: "Done", Level: "info"}}, + {ActionBase: config_loader.ActionBase{Name: "log1", Log: &config_loader.LogAction{Message: "Done", Level: "info"}}}, }, }, expectedResults: 1, diff --git a/internal/executor/precondition_executor.go b/internal/executor/precondition_executor.go index 86808e2..998f52e 100644 --- a/internal/executor/precondition_executor.go +++ b/internal/executor/precondition_executor.go @@ -185,9 +185,17 @@ func (pe *PreconditionExecutor) executePrecondition(ctx context.Context, precond // Log individual condition results for _, cr := range condResult.Results { if cr.Matched { - pe.log.Debugf(ctx, "Condition: %s %s %v = %v ✓", cr.Field, cr.Operator, cr.ExpectedValue, cr.FieldValue) + if cr.Operator == criteria.OperatorExists { + pe.log.Debugf(ctx, "Condition: %s %s = %v ✓", cr.Field, cr.Operator, cr.FieldValue) + } else { + pe.log.Debugf(ctx, "Condition: %s %s %v = %v ✓", cr.Field, cr.Operator, cr.ExpectedValue, cr.FieldValue) + } } else { - pe.log.Infof(ctx, "Condition FAILED: %s %s %v (actual: %v)", cr.Field, cr.Operator, cr.ExpectedValue, cr.FieldValue) + if cr.Operator == criteria.OperatorExists { + pe.log.Infof(ctx, "Condition FAILED: %s %s (field is nil or empty)", cr.Field, cr.Operator) + } else { + pe.log.Infof(ctx, "Condition FAILED: %s %s %v (actual: %v)", cr.Field, cr.Operator, cr.ExpectedValue, cr.FieldValue) + } } } diff --git a/test/integration/config-loader/testdata/adapter_config_valid.yaml b/test/integration/config-loader/testdata/adapter_config_valid.yaml index d004ced..b00c31b 100644 --- a/test/integration/config-loader/testdata/adapter_config_valid.yaml +++ b/test/integration/config-loader/testdata/adapter_config_valid.yaml @@ -78,7 +78,6 @@ spec: value: ["aws", "gcp", "azure"] - field: "vpcId" operator: "exists" - value: true - name: "validationCheck" # Valid CEL expression diff --git a/test/integration/executor/executor_integration_test.go b/test/integration/executor/executor_integration_test.go index cb01b3b..695ba1d 100644 --- a/test/integration/executor/executor_integration_test.go +++ b/test/integration/executor/executor_integration_test.go @@ -428,16 +428,18 @@ func TestExecutor_CELExpressionEvaluation(t *testing.T) { config := createTestConfig(mockAPI.URL()) config.Spec.Preconditions = []config_loader.Precondition{ { - Name: "clusterStatus", - APICall: &config_loader.APICall{ - Method: "GET", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "clusterStatus", + APICall: &config_loader.APICall{ + Method: "GET", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", + Timeout: "5s", + }, }, Capture: []config_loader.CaptureField{ - {Name: "clusterName", Field: "metadata.name"}, - {Name: "clusterPhase", Field: "status.phase"}, - {Name: "nodeCount", Field: "spec.node_count"}, + {Name: "clusterName", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "metadata.name"}}, + {Name: "clusterPhase", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "status.phase"}}, + {Name: "nodeCount", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "spec.node_count"}}, }, // Use CEL expression instead of structured conditions Expression: `clusterPhase == "Ready" && nodeCount >= 3`, @@ -863,28 +865,34 @@ func TestExecutor_LogAction(t *testing.T) { Preconditions: []config_loader.Precondition{ { // Log action only - no API call or conditions - Name: "logStart", - Log: &config_loader.LogAction{ - Message: "Starting processing for cluster {{ .clusterId }}", - Level: "info", + ActionBase: config_loader.ActionBase{ + Name: "logStart", + Log: &config_loader.LogAction{ + Message: "Starting processing for cluster {{ .clusterId }}", + Level: "info", + }, }, }, { // Log action before API call - Name: "logBeforeAPICall", - Log: &config_loader.LogAction{ - Message: "About to check cluster status for {{ .clusterId }}", - Level: "debug", + ActionBase: config_loader.ActionBase{ + Name: "logBeforeAPICall", + Log: &config_loader.LogAction{ + Message: "About to check cluster status for {{ .clusterId }}", + Level: "debug", + }, }, }, { - Name: "checkCluster", - APICall: &config_loader.APICall{ - Method: "GET", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", + ActionBase: config_loader.ActionBase{ + Name: "checkCluster", + APICall: &config_loader.APICall{ + Method: "GET", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", + }, }, Capture: []config_loader.CaptureField{ - {Name: "clusterPhase", Field: "status.phase"}, + {Name: "clusterPhase", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "status.phase"}}, }, Conditions: []config_loader.Condition{ {Field: "clusterPhase", Operator: "equals", Value: "Ready"}, @@ -895,18 +903,22 @@ func TestExecutor_LogAction(t *testing.T) { PostActions: []config_loader.PostAction{ { // Log action in post-actions - Name: "logCompletion", - Log: &config_loader.LogAction{ - Message: "Completed processing cluster {{ .clusterId }} with resource {{ .resourceId }}", - Level: "info", + ActionBase: config_loader.ActionBase{ + Name: "logCompletion", + Log: &config_loader.LogAction{ + Message: "Completed processing cluster {{ .clusterId }} with resource {{ .resourceId }}", + Level: "info", + }, }, }, { // Log with warning level - Name: "logWarning", - Log: &config_loader.LogAction{ - Message: "This is a warning for cluster {{ .clusterId }}", - Level: "warning", + ActionBase: config_loader.ActionBase{ + Name: "logWarning", + Log: &config_loader.LogAction{ + Message: "This is a warning for cluster {{ .clusterId }}", + Level: "warning", + }, }, }, }, @@ -1097,14 +1109,16 @@ func TestExecutor_ExecutionError_CELAccess(t *testing.T) { }, Preconditions: []config_loader.Precondition{ { - Name: "clusterStatus", - APICall: &config_loader.APICall{ - Method: "GET", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "clusterStatus", + APICall: &config_loader.APICall{ + Method: "GET", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", + Timeout: "5s", + }, }, Capture: []config_loader.CaptureField{ - {Name: "clusterPhase", Field: "status.phase"}, + {Name: "clusterPhase", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "status.phase"}}, }, Conditions: []config_loader.Condition{ {Field: "clusterPhase", Operator: "equals", Value: "Ready"}, @@ -1145,12 +1159,14 @@ func TestExecutor_ExecutionError_CELAccess(t *testing.T) { }, PostActions: []config_loader.PostAction{ { - Name: "reportError", - APICall: &config_loader.APICall{ - Method: "POST", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/error-report", - Body: "{{ .errorReportPayload }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "reportError", + APICall: &config_loader.APICall{ + Method: "POST", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/error-report", + Body: "{{ .errorReportPayload }}", + Timeout: "5s", + }, }, }, }, @@ -1262,7 +1278,7 @@ func TestExecutor_PayloadBuildFailure(t *testing.T) { }, Preconditions: []config_loader.Precondition{ { - Name: "simpleCheck", + ActionBase: config_loader.ActionBase{Name: "simpleCheck"}, Conditions: []config_loader.Condition{ {Field: "clusterId", Operator: "equals", Value: "test-cluster"}, }, @@ -1283,12 +1299,14 @@ func TestExecutor_PayloadBuildFailure(t *testing.T) { }, PostActions: []config_loader.PostAction{ { - Name: "shouldNotExecute", - APICall: &config_loader.APICall{ - Method: "POST", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/status", - Body: "{{ .badPayload }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "shouldNotExecute", + APICall: &config_loader.APICall{ + Method: "POST", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/status", + Body: "{{ .badPayload }}", + Timeout: "5s", + }, }, }, }, diff --git a/test/integration/executor/executor_k8s_integration_test.go b/test/integration/executor/executor_k8s_integration_test.go index 478af98..274dbff 100644 --- a/test/integration/executor/executor_k8s_integration_test.go +++ b/test/integration/executor/executor_k8s_integration_test.go @@ -179,17 +179,19 @@ func createK8sTestConfig(apiBaseURL, testNamespace string) *config_loader.Adapte }, Preconditions: []config_loader.Precondition{ { - Name: "clusterStatus", - APICall: &config_loader.APICall{ - Method: "GET", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "clusterStatus", + APICall: &config_loader.APICall{ + Method: "GET", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}", + Timeout: "5s", + }, }, Capture: []config_loader.CaptureField{ - {Name: "clusterName", Field: "metadata.name"}, - {Name: "clusterPhase", Field: "status.phase"}, - {Name: "region", Field: "spec.region"}, - {Name: "cloudProvider", Field: "spec.provider"}, + {Name: "clusterName", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "metadata.name"}}, + {Name: "clusterPhase", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "status.phase"}}, + {Name: "region", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "spec.region"}}, + {Name: "cloudProvider", FieldExpressionDef: config_loader.FieldExpressionDef{Field: "spec.provider"}}, }, Conditions: []config_loader.Condition{ {Field: "clusterPhase", Operator: "in", Value: []interface{}{"Provisioning", "Installing", "Ready"}}, @@ -280,12 +282,14 @@ func createK8sTestConfig(apiBaseURL, testNamespace string) *config_loader.Adapte }, PostActions: []config_loader.PostAction{ { - Name: "reportClusterStatus", - APICall: &config_loader.APICall{ - Method: "POST", - URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/status", - Body: "{{ .clusterStatusPayload }}", - Timeout: "5s", + ActionBase: config_loader.ActionBase{ + Name: "reportClusterStatus", + APICall: &config_loader.APICall{ + Method: "POST", + URL: "{{ .hyperfleetApiBaseUrl }}/api/{{ .hyperfleetApiVersion }}/clusters/{{ .clusterId }}/status", + Body: "{{ .clusterStatusPayload }}", + Timeout: "5s", + }, }, }, }, diff --git a/test/testdata/adapter_config_valid.yaml b/test/testdata/adapter_config_valid.yaml index 1d91cc6..5e53b97 100644 --- a/test/testdata/adapter_config_valid.yaml +++ b/test/testdata/adapter_config_valid.yaml @@ -73,7 +73,6 @@ spec: value: ["aws", "gcp", "azure"] - field: "vpcId" operator: "exists" - value: true - name: "validationCheck" # Valid CEL expression