From a2fe0b9fdce6d6dda03388dd8acb235352de39ba Mon Sep 17 00:00:00 2001 From: Brian Kane Date: Thu, 12 Feb 2026 17:09:48 +0000 Subject: [PATCH] Fix: Backport commits to 1.10 (#7040) --- hack/debug-webhook-setup.sh | 111 ++- pkg/appfile/validate.go | 214 +++++ pkg/appfile/validate_test.go | 812 ++++++++++++++++++ .../application/application_metrics.go | 32 +- .../application/application_metrics_test.go | 79 +- 5 files changed, 1230 insertions(+), 18 deletions(-) diff --git a/hack/debug-webhook-setup.sh b/hack/debug-webhook-setup.sh index 131b5f9dd..f8504374d 100755 --- a/hack/debug-webhook-setup.sh +++ b/hack/debug-webhook-setup.sh @@ -75,12 +75,61 @@ generate_certificates() { # Generate server private key openssl genrsa -out ${CERT_DIR}/tls.key 2048 - # Get host IP for Docker internal network - # NOTE: 192.168.5.2 is the standard k3d host gateway IP that allows containers to reach the host machine + # Auto-detect host IP for Docker/k3d internal network # This is only for local k3d development environments - DO NOT use this script in production # With failurePolicy: Fail, an unreachable webhook can block CRD operations cluster-wide - HOST_IP="192.168.5.2" - LOCAL_IP=$(ifconfig | grep "inet " | grep -v 127.0.0.1 | head -1 | awk '{print $2}') + + # Try to detect k3d cluster + K3D_CLUSTER=$(kubectl config current-context | grep -o 'k3d-[^@]*' | sed 's/k3d-//' || echo "") + + if [ -n "$K3D_CLUSTER" ]; then + echo "Detected k3d cluster: $K3D_CLUSTER" + + # Check if k3d is using host network + NETWORK_MODE=$(docker inspect "k3d-${K3D_CLUSTER}-server-0" 2>/dev/null | grep -o '"NetworkMode": "[^"]*"' | cut -d'"' -f4 || echo "") + + if [ "$NETWORK_MODE" = "host" ]; then + # Host network mode - detect OS + if [ "$(uname)" = "Darwin" ]; then + # macOS with Docker Desktop - use host.docker.internal + echo "Detected k3d with --network host on macOS, using host.docker.internal" + HOST_IP="host.docker.internal" + else + # Linux - true host networking works + echo "Detected k3d with --network host, using localhost" + HOST_IP="127.0.0.1" + fi + else + # Bridge network mode - get gateway IP + NETWORK_NAME="k3d-${K3D_CLUSTER}" + HOST_IP=$(docker network inspect "$NETWORK_NAME" -f '{{range .IPAM.Config}}{{.Gateway}}{{end}}' 2>/dev/null || echo "") + + if [ -z "$HOST_IP" ]; then + # Fallback to common k3d gateway IPs + echo "Could not detect gateway IP, trying common defaults..." + if docker exec "k3d-${K3D_CLUSTER}-server-0" getent hosts host.k3d.internal 2>/dev/null | awk '{print $1}' | grep -q .; then + HOST_IP=$(docker exec "k3d-${K3D_CLUSTER}-server-0" cat /etc/hosts | grep host.k3d.internal | awk '{print $1}') + else + HOST_IP="172.18.0.1" + fi + fi + + echo "Detected k3d with bridge network, using gateway IP: $HOST_IP" + fi + else + # Not k3d, use default + echo "Not using k3d, defaulting to 192.168.5.2" + HOST_IP="192.168.5.2" + fi + + # Get local machine IP for SANs (optional, for reference) + if command -v ifconfig &> /dev/null; then + LOCAL_IP=$(ifconfig | grep "inet " | grep -v 127.0.0.1 | head -1 | awk '{print $2}') + elif command -v ip &> /dev/null; then + LOCAL_IP=$(ip -4 addr show | grep -oP '(?<=inet\s)\d+(\.\d+){3}' | grep -v 127.0.0.1 | head -1) + else + LOCAL_IP="" + fi # Create certificate config with SANs cat > /tmp/webhook.conf << EOF @@ -98,11 +147,26 @@ DNS.2 = vela-webhook.${NAMESPACE}.svc DNS.3 = vela-webhook.${NAMESPACE}.svc.cluster.local DNS.4 = *.${NAMESPACE}.svc DNS.5 = *.${NAMESPACE}.svc.cluster.local +DNS.6 = host.k3d.internal +DNS.7 = host.docker.internal +DNS.8 = host.lima.internal IP.1 = 127.0.0.1 -IP.2 = ${HOST_IP} -IP.3 = ${LOCAL_IP} EOF + # Add HOST_IP - check if it's a hostname or IP + if [[ "$HOST_IP" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + # It's an IP address + echo "IP.2 = ${HOST_IP}" >> /tmp/webhook.conf + else + # It's a hostname - already covered by DNS SANs above + echo "# HOST_IP is hostname: ${HOST_IP} (already in DNS SANs)" >> /tmp/webhook.conf + fi + + # Add LOCAL_IP to SANs only if detected and is an IP + if [ -n "$LOCAL_IP" ] && [[ "$LOCAL_IP" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "IP.3 = ${LOCAL_IP}" >> /tmp/webhook.conf + fi + # Generate certificate request openssl req -new -key ${CERT_DIR}/tls.key -out /tmp/server.csr \ -subj "/CN=vela-webhook.${NAMESPACE}.svc" -config /tmp/webhook.conf @@ -226,19 +290,38 @@ show_next_steps() { echo "Webhook debugging setup complete!" echo "=========================================" echo -e "${NC}" + + echo "Configuration:" + echo " - Webhook URL: https://${HOST_IP}:9445" + echo " - Certificate directory: ${CERT_DIR}" + + if [ -n "$K3D_CLUSTER" ]; then + echo " - k3d cluster: ${K3D_CLUSTER}" + if [ "$NETWORK_MODE" = "host" ]; then + echo " - Network mode: host (using ${HOST_IP})" + else + echo " - Network mode: bridge (using gateway ${HOST_IP})" + fi + fi + + echo "" echo "Next steps:" - echo "1. Open VS Code" + echo "1. Open your IDE (VS Code, GoLand, etc.)" echo "2. Set breakpoints in webhook validation code:" - echo " - pkg/webhook/utils/utils.go:141" - echo " - pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler.go:74" - echo "3. Press F5 and select 'Debug Webhook Validation'" - echo "4. Wait for webhook server to start (port 9445)" + echo " - pkg/webhook/core.oam.dev/v1beta1/application/validating_handler.go:66" + echo " - pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler.go:74" + echo "3. Start debugging cmd/core/main.go with arguments:" + echo " --use-webhook=true" + echo " --webhook-port=9445" + echo " --webhook-cert-dir=${CERT_DIR}" + echo " --leader-elect=false" + echo "4. Wait for webhook server to start" echo "5. Test with kubectl apply commands" echo "" - echo -e "${YELLOW}Test command (should be rejected):${NC}" - echo 'kubectl apply -f test/webhook-test-invalid.yaml' + echo -e "${YELLOW}Test command:${NC}" + echo 'kubectl apply -f ' echo "" - echo -e "${GREEN}The webhook will reject ComponentDefinitions with non-existent CRDs${NC}" + echo -e "${GREEN}Your breakpoints will hit when kubectl applies resources!${NC}" } # Main execution diff --git a/pkg/appfile/validate.go b/pkg/appfile/validate.go index 84c0d2492..7da0fa0b6 100644 --- a/pkg/appfile/validate.go +++ b/pkg/appfile/validate.go @@ -55,7 +55,24 @@ func (p *Parser) ValidateCUESchematicAppfile(a *Appfile) error { } } + // Collect workflow-supplied params for this component upfront + workflowParams := getWorkflowAndPolicySuppliedParams(a) + + // Only augment if component has traits AND workflow supplies params (issue 7022) + originalParams := wl.Params + if len(wl.Traits) > 0 && len(workflowParams) > 0 { + shouldSkip, augmented := p.augmentComponentParamsForValidation(wl, workflowParams, ctxData) + if shouldSkip { + // Component has complex validation that can't be handled, skip trait validation + fmt.Printf("INFO: Skipping trait validation for component %q due to workflow-supplied parameters with complex validation\n", wl.Name) + continue + } + wl.Params = augmented + } + pCtx, err := newValidationProcessContext(wl, ctxData) + wl.Params = originalParams // Restore immediately + if err != nil { return errors.WithMessagef(err, "cannot create the validation process context of app=%s in namespace=%s", a.Name, a.Namespace) } @@ -329,3 +346,200 @@ func validateAuxiliaryNameUnique() process.AuxiliaryHook { return nil }) } + +// getWorkflowAndPolicySuppliedParams returns a set of parameter keys that will be +// supplied by workflow steps or override policies at runtime. +func getWorkflowAndPolicySuppliedParams(app *Appfile) map[string]bool { + result := make(map[string]bool) + + // Collect from workflow step inputs + for _, step := range app.WorkflowSteps { + for _, in := range step.Inputs { + result[in.ParameterKey] = true + } + } + + // Collect from override policies + for _, p := range app.Policies { + if p.Type != "override" { + continue + } + + var spec overrideSpec + if err := json.Unmarshal(p.Properties.Raw, &spec); err != nil { + continue // Skip if we can't parse + } + + for _, c := range spec.Components { + if len(c.Properties) == 0 { + continue + } + + flat, err := flatten.Flatten(c.Properties, "", flatten.DotStyle) + if err != nil { + continue // Skip if we can't flatten + } + + for k := range flat { + result[k] = true + } + } + } + + return result +} + +// getDefaultForMissingParameter checks if a parameter can be defaulted for validation +// and returns an appropriate placeholder value. +func getDefaultForMissingParameter(v cue.Value) (bool, any) { + if v.IsConcrete() { + return true, nil + } + + if defaultVal, hasDefault := v.Default(); hasDefault { + return true, defaultVal + } + + // Use Expr() to inspect the operation tree for complex validation + op, args := v.Expr() + + switch op { + case cue.NoOp, cue.SelectorOp: + // No operation or field selector - simple type + // Use IncompleteKind for non-concrete values to get the correct type + return true, getTypeDefault(v.IncompleteKind()) + + case cue.AndOp: + // Conjunction (e.g., int & >0 & <100) + if len(args) > 1 { + // Check if any arg is NOT just a basic kind (indicates complex validation) + for _, arg := range args { + if arg.Kind() == cue.BottomKind { + return false, nil + } + } + } + return true, getTypeDefault(v.IncompleteKind()) + + case cue.OrOp: + // Disjunction (e.g., "value1" | "value2" | "value3") - likely an enum + if len(args) > 0 { + firstVal := args[0] + if firstVal.IsConcrete() { + var result any + if err := firstVal.Decode(&result); err == nil { + return true, result + } + } + } + return false, nil + + default: + return false, nil + } +} + +// getTypeDefault returns a simple default value based on the CUE Kind. +func getTypeDefault(kind cue.Kind) any { + switch kind { + case cue.StringKind: + return "__workflow_supplied__" + case cue.FloatKind: + return 0.0 + case cue.IntKind, cue.NumberKind: + return 0 + case cue.BoolKind: + return false + case cue.ListKind: + return []any{} + case cue.StructKind: + return map[string]any{} + default: + return "__workflow_supplied__" + } +} + +// augmentComponentParamsForValidation checks if workflow-supplied parameters +// need to be augmented for trait validation. Returns (shouldSkip, augmentedParams). +// If shouldSkip=true, the component has complex validation and should skip trait validation. +// If shouldSkip=false, augmentedParams contains the original params plus simple defaults. +func (p *Parser) augmentComponentParamsForValidation(wl *Component, workflowParams map[string]bool, ctxData velaprocess.ContextData) (bool, map[string]any) { + // Build CUE value to inspect the component's parameter schema + ctx := velaprocess.NewContext(ctxData) + baseCtx, err := ctx.BaseContextFile() + if err != nil { + return false, wl.Params // Can't inspect, proceed normally + } + + paramSnippet, err := cueParamBlock(wl.Params) + if err != nil { + return false, wl.Params + } + + cueSrc := strings.Join([]string{ + renderTemplate(wl.FullTemplate.TemplateStr), + paramSnippet, + baseCtx, + }, "\n") + + val, err := cuex.DefaultCompiler.Get().CompileString(ctx.GetCtx(), cueSrc) + if err != nil { + return false, wl.Params // Can't compile, proceed normally + } + + // Get the parameter schema + paramVal := val.LookupPath(value.FieldPath(velaprocess.ParameterFieldName)) + + // Collect default values for workflow-supplied params that are missing + workflowParamDefaults := make(map[string]any) + + for paramKey := range workflowParams { + // Skip if already provided + if _, exists := wl.Params[paramKey]; exists { + continue + } + + // Check the field in the schema + fieldVal := paramVal.LookupPath(cue.ParsePath(paramKey)) + if !fieldVal.Exists() { + continue // Not a parameter field + } + + canDefault, defaultVal := getDefaultForMissingParameter(fieldVal) + if !canDefault { + // complex validation - skip + return true, nil + } + + if defaultVal != nil { + workflowParamDefaults[paramKey] = defaultVal + } + } + + if len(workflowParamDefaults) == 0 { + return false, wl.Params + } + + // Create augmented params map + augmented := make(map[string]any) + for k, v := range wl.Params { + augmented[k] = v + } + for k, v := range workflowParamDefaults { + augmented[k] = v + } + + fmt.Printf("INFO: Augmented component %q with workflow-supplied defaults for trait validation: %v\n", + wl.Name, getMapKeys(workflowParamDefaults)) + + return false, augmented +} + +// getMapKeys returns the keys from a map as a slice +func getMapKeys(m map[string]any) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} diff --git a/pkg/appfile/validate_test.go b/pkg/appfile/validate_test.go index 4fe61bc7b..af84177b9 100644 --- a/pkg/appfile/validate_test.go +++ b/pkg/appfile/validate_test.go @@ -17,11 +17,20 @@ limitations under the License. package appfile import ( + "testing" + + "cuelang.org/go/cue" + workflowv1alpha1 "github.com/kubevela/workflow/api/v1alpha1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/cue/definition" + "github.com/oam-dev/kubevela/pkg/features" ) var _ = Describe("Test validate CUE schematic Appfile", func() { @@ -262,3 +271,806 @@ var _ = Describe("Test ValidateComponentParams", func() { }), ) }) + +func TestValidationHelpers(t *testing.T) { + t.Run("renderTemplate", func(t *testing.T) { + tmpl := "output: {}" + expected := "output: {}\ncontext: _\nparameter: _\n" + assert.Equal(t, expected, renderTemplate(tmpl)) + }) + + t.Run("cueParamBlock", func(t *testing.T) { + t.Run("should handle empty params", func(t *testing.T) { + out, err := cueParamBlock(map[string]any{}) + assert.NoError(t, err) + assert.Equal(t, "parameter: {}", out) + }) + + t.Run("should handle valid params", func(t *testing.T) { + params := map[string]any{"key": "value"} + out, err := cueParamBlock(params) + assert.NoError(t, err) + assert.Equal(t, `parameter: {"key":"value"}`, out) + }) + + t.Run("should return error for unmarshallable params", func(t *testing.T) { + params := map[string]any{"key": make(chan int)} + _, err := cueParamBlock(params) + assert.Error(t, err) + }) + }) + + t.Run("filterMissing", func(t *testing.T) { + t.Run("should filter missing keys", func(t *testing.T) { + keys := []string{"a", "b.c", "d"} + provided := map[string]any{ + "a": 1, + "b": map[string]any{ + "c": 2, + }, + } + out, err := filterMissing(keys, provided) + assert.NoError(t, err) + assert.Equal(t, []string{"d"}, out) + }) + + t.Run("should handle no missing keys", func(t *testing.T) { + keys := []string{"a"} + provided := map[string]any{"a": 1} + out, err := filterMissing(keys, provided) + assert.NoError(t, err) + assert.Empty(t, out) + }) + }) + + t.Run("requiredFields", func(t *testing.T) { + t.Run("should identify required fields", func(t *testing.T) { + cueStr := ` + parameter: { + name: string + age: int + nested: { + field1: string + field2: bool + } + } + ` + var r cue.Runtime + inst, err := r.Compile("", cueStr) + assert.NoError(t, err) + val := inst.Value() + paramVal := val.LookupPath(cue.ParsePath("parameter")) + + fields, err := requiredFields(paramVal) + assert.NoError(t, err) + assert.ElementsMatch(t, []string{"name", "age", "nested.field1", "nested.field2"}, fields) + }) + + t.Run("should ignore optional and default fields", func(t *testing.T) { + cueStr := ` + parameter: { + name: string + age?: int + location: string | *"unknown" + nested: { + field1: string + field2?: bool + } + } + ` + var r cue.Runtime + inst, err := r.Compile("", cueStr) + assert.NoError(t, err) + val := inst.Value() + paramVal := val.LookupPath(cue.ParsePath("parameter")) + + fields, err := requiredFields(paramVal) + assert.NoError(t, err) + assert.ElementsMatch(t, []string{"name", "nested.field1"}, fields) + }) + }) +} + +func TestEnforceRequiredParams(t *testing.T) { + var r cue.Runtime + cueStr := ` + parameter: { + image: string + replicas: int + port: int + data: { + key: string + value: string + } + } + ` + inst, err := r.Compile("", cueStr) + assert.NoError(t, err) + root := inst.Value() + + t.Run("should pass if all params are provided directly", func(t *testing.T) { + params := map[string]any{ + "image": "nginx", + "replicas": 2, + "port": 80, + "data": map[string]any{ + "key": "k", + "value": "v", + }, + } + app := &Appfile{} + err := enforceRequiredParams(root, params, app) + assert.NoError(t, err) + }) + + t.Run("should fail if params are missing", func(t *testing.T) { + params := map[string]any{ + "image": "nginx", + } + app := &Appfile{} + err := enforceRequiredParams(root, params, app) + assert.Error(t, err) + assert.Contains(t, err.Error(), "missing parameters: replicas,port,data.key,data.value") + }) +} + +func TestParser_ValidateCUESchematicAppfile(t *testing.T) { + assert.NoError(t, utilfeature.DefaultMutableFeatureGate.Set(string(features.EnableCueValidation)+"=true")) + t.Cleanup(func() { + assert.NoError(t, utilfeature.DefaultMutableFeatureGate.Set(string(features.EnableCueValidation)+"=false")) + }) + + t.Run("should validate a valid CUE schematic appfile", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-comp", + Type: "worker", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "image": "nginx", + }, + FullTemplate: &Template{ + TemplateStr: ` + parameter: { + image: string + } + output: { + apiVersion: "apps/v1" + kind: "Deployment" + spec: { + template: { + spec: { + containers: [{ + name: "my-container" + image: parameter.image + }] + } + } + } + } + `, + }, + engine: definition.NewWorkloadAbstractEngine("my-comp"), + Traits: []*Trait{ + { + Name: "my-trait", + CapabilityCategory: types.CUECategory, + Template: ` + parameter: { + domain: string + } + patch: {} + `, + Params: map[string]any{ + "domain": "example.com", + }, + engine: definition.NewTraitAbstractEngine("my-trait"), + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err) + }) + + t.Run("should return error for invalid trait evaluation", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-comp", + Type: "worker", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "image": "nginx", + }, + FullTemplate: &Template{ + TemplateStr: ` + parameter: { + image: string + } + output: { + apiVersion: "apps/v1" + kind: "Deployment" + } + `, + }, + engine: definition.NewWorkloadAbstractEngine("my-comp"), + Traits: []*Trait{ + { + Name: "my-trait", + CapabilityCategory: types.CUECategory, + Template: ` + // invalid CUE template + parameter: { + domain: string + } + patch: { + invalid: { + } + `, + Params: map[string]any{ + "domain": "example.com", + }, + engine: definition.NewTraitAbstractEngine("my-trait"), + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "cannot evaluate trait \"my-trait\"") + }) + + t.Run("should return error for missing parameters", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-comp", + Type: "worker", + CapabilityCategory: types.CUECategory, + Params: map[string]any{}, // no params provided + FullTemplate: &Template{ + TemplateStr: ` + parameter: { + image: string + } + output: { + apiVersion: "apps/v1" + kind: "Deployment" + } + `, + }, + engine: definition.NewWorkloadAbstractEngine("my-comp"), + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.Error(t, err) + assert.Contains(t, err.Error(), "missing parameters: image") + }) + + t.Run("should skip non-CUE components", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-comp", + Type: "helm", + CapabilityCategory: types.TerraformCategory, + }, + }, + } + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err) + }) +} + +// TestValidateCUESchematicAppfile_WorkflowSuppliedParams tests validation with workflow-supplied parameters (issue #7022) +func TestValidateCUESchematicAppfile_WorkflowSuppliedParams(t *testing.T) { + assert.NoError(t, utilfeature.DefaultMutableFeatureGate.Set(string(features.EnableCueValidation)+"=true")) + t.Cleanup(func() { + assert.NoError(t, utilfeature.DefaultMutableFeatureGate.Set(string(features.EnableCueValidation)+"=false")) + }) + + componentTemplate := ` + parameter: { + image: string + port: int | *80 + } + output: { + apiVersion: "apps/v1" + kind: "Deployment" + spec: { + template: { + spec: { + containers: [{ + name: "main" + image: parameter.image + ports: [{ + containerPort: parameter.port + }] + }] + } + } + } + } + ` + + traitTemplate := ` + parameter: { + key: string + value: string + } + patch: { + metadata: { + labels: { + (parameter.key): parameter.value + } + } + } + ` + + t.Run("workflow supplies param - NO traits - should PASS", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: componentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-microservice", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + { + From: "dynamicValue", + ParameterKey: "image", + }, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should pass when workflow supplies missing param and NO traits present") + }) + + t.Run("workflow supplies param - WITH traits - should PASS", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: componentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "release", + "value": "stable", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-microservice", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + { + From: "dynamicValue", + ParameterKey: "image", + }, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should pass when workflow supplies missing param even WITH traits") + }) + + t.Run("workflow supplies param with ENUM - should use first enum value", func(t *testing.T) { + enumComponentTemplate := ` + parameter: { + image: "nginx:latest" | "apache:latest" | "httpd:latest" + port: int | *80 + } + output: { + apiVersion: "apps/v1" + kind: "Deployment" + spec: { + template: { + spec: { + containers: [{ + name: "main" + image: parameter.image + ports: [{ + containerPort: parameter.port + }] + }] + } + } + } + } + ` + + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: enumComponentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "release", + "value": "stable", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-microservice", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + { + From: "dynamicValue", + ParameterKey: "image", + }, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should use first enum value as default") + }) + + t.Run("param missing everywhere - should FAIL", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: componentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "release", + "value": "stable", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.Error(t, err, "Should fail when param is missing everywhere") + assert.Contains(t, err.Error(), "missing parameters: image") + }) + + t.Run("override policy supplies param - WITH traits - should PASS", func(t *testing.T) { + policyJSON := `{ + "components": [{ + "properties": { + "image": "nginx:1.20" + } + }] + }` + + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: componentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "release", + "value": "stable", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + Policies: []v1beta1.AppPolicy{ + { + Name: "override-policy", + Type: "override", + Properties: &runtime.RawExtension{ + Raw: []byte(policyJSON), + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should pass when override policy supplies missing param") + }) + + t.Run("workflow supplies different param types - should use correct defaults", func(t *testing.T) { + multiTypeTemplate := ` + parameter: { + count: int + enabled: bool + tags: [...string] + port: int | *80 + } + output: { + apiVersion: "v1" + kind: "ConfigMap" + data: { + count: "\(parameter.count)" + enabled: "\(parameter.enabled)" + port: "\(parameter.port)" + } + metadata: { + labels: { + for i, tag in parameter.tags { + "tag-\(i)": tag + } + } + } + } + ` + + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-config", + Type: "raw", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "port": 80, + }, + FullTemplate: &Template{ + TemplateStr: multiTypeTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-config"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "env", + "value": "test", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-config", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + {From: "dynamicCount", ParameterKey: "count"}, + {From: "dynamicEnabled", ParameterKey: "enabled"}, + {From: "dynamicTags", ParameterKey: "tags"}, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should handle int, bool, list types with correct defaults") + }) + + t.Run("workflow supplies param with numeric bounds - should skip validation", func(t *testing.T) { + // Component with complex validation that can't be easily defaulted + complexTemplate := ` + parameter: { + port: int & >1024 & <65535 + image: string + } + output: { + apiVersion: "v1" + kind: "Service" + spec: { + ports: [{ + port: parameter.port + }] + } + } + ` + + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-service", + Type: "service", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "image": "nginx:latest", + }, + FullTemplate: &Template{ + TemplateStr: complexTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-service"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "version", + "value": "v1", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-service", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + {From: "dynamicPort", ParameterKey: "port"}, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + // Should pass by skipping validation due to complex constraints + assert.NoError(t, err, "Should skip validation when complex constraints cannot be satisfied") + }) + + t.Run("workflow param already provided in component - should not augment", func(t *testing.T) { + appfile := &Appfile{ + Name: "test-app", + Namespace: "test-ns", + ParsedComponents: []*Component{ + { + Name: "my-webservice", + Type: "webservice", + CapabilityCategory: types.CUECategory, + Params: map[string]any{ + "image": "custom-image:v1.0", + "port": 8080, + }, + FullTemplate: &Template{ + TemplateStr: componentTemplate, + }, + engine: definition.NewWorkloadAbstractEngine("my-webservice"), + Traits: []*Trait{ + { + Name: "labels", + CapabilityCategory: types.CUECategory, + Template: traitTemplate, + Params: map[string]any{ + "key": "app", + "value": "myapp", + }, + engine: definition.NewTraitAbstractEngine("labels"), + }, + }, + }, + }, + WorkflowSteps: []workflowv1alpha1.WorkflowStep{ + { + WorkflowStepBase: workflowv1alpha1.WorkflowStepBase{ + Name: "apply-webservice", + Type: "apply-component", + Inputs: workflowv1alpha1.StepInputs{ + {From: "dynamicImage", ParameterKey: "image"}, + }, + }, + }, + }, + } + + p := &Parser{} + err := p.ValidateCUESchematicAppfile(appfile) + assert.NoError(t, err, "Should use existing param value, not augment from workflow") + }) +} diff --git a/pkg/controller/core.oam.dev/v1beta1/application/application_metrics.go b/pkg/controller/core.oam.dev/v1beta1/application/application_metrics.go index 780fd6b98..2174c07b4 100644 --- a/pkg/controller/core.oam.dev/v1beta1/application/application_metrics.go +++ b/pkg/controller/core.oam.dev/v1beta1/application/application_metrics.go @@ -42,7 +42,7 @@ func (r *Reconciler) updateMetricsAndLog(_ context.Context, app *v1beta1.Applica updatePhaseMetrics(app) workflowStatus := buildWorkflowStatus(app.Status.Workflow) - serviceDetails := buildServiceDetails(app.Status.Services) + serviceDetails := buildServiceDetails(app, app.Status.Services) logApplicationStatus(app, healthStatus, workflowStatus, serviceDetails) } @@ -106,13 +106,24 @@ func buildWorkflowStatus(workflow *common.WorkflowStatus) map[string]interface{} } } +// getComponentType looks up the component type from the application spec +func getComponentType(app *v1beta1.Application, componentName string) string { + for _, comp := range app.Spec.Components { + if comp.Name == componentName { + return comp.Type + } + } + return "" +} + // buildServiceDetails builds service details for logging -func buildServiceDetails(services []common.ApplicationComponentStatus) []map[string]interface{} { +func buildServiceDetails(app *v1beta1.Application, services []common.ApplicationComponentStatus) []map[string]interface{} { serviceDetails := make([]map[string]interface{}, 0, len(services)) for _, svc := range services { svcDetails := map[string]interface{}{ "name": svc.Name, + "type": getComponentType(app, svc.Name), "namespace": svc.Namespace, "cluster": svc.Cluster, "healthy": svc.Healthy, @@ -121,6 +132,23 @@ func buildServiceDetails(services []common.ApplicationComponentStatus) []map[str if len(svc.Details) > 0 { svcDetails["details"] = svc.Details } + if len(svc.Traits) > 0 { + traits := make([]map[string]interface{}, 0, len(svc.Traits)) + for _, trait := range svc.Traits { + traitDetails := map[string]interface{}{ + "type": trait.Type, + "healthy": trait.Healthy, + } + if trait.Message != "" { + traitDetails["message"] = trait.Message + } + if len(trait.Details) > 0 { + traitDetails["details"] = trait.Details + } + traits = append(traits, traitDetails) + } + svcDetails["traits"] = traits + } serviceDetails = append(serviceDetails, svcDetails) } diff --git a/pkg/controller/core.oam.dev/v1beta1/application/application_metrics_test.go b/pkg/controller/core.oam.dev/v1beta1/application/application_metrics_test.go index e9f6026e4..cfcbcfaee 100644 --- a/pkg/controller/core.oam.dev/v1beta1/application/application_metrics_test.go +++ b/pkg/controller/core.oam.dev/v1beta1/application/application_metrics_test.go @@ -184,16 +184,28 @@ func TestBuildWorkflowStatus(t *testing.T) { func TestBuildServiceDetails(t *testing.T) { tests := []struct { name string + app *v1beta1.Application services []common.ApplicationComponentStatus want []map[string]interface{} }{ { - name: "empty services", + name: "empty services", + app: &v1beta1.Application{ + Spec: v1beta1.ApplicationSpec{}, + }, services: []common.ApplicationComponentStatus{}, want: []map[string]interface{}{}, }, { name: "services with details", + app: &v1beta1.Application{ + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{ + {Name: "web", Type: "webservice"}, + {Name: "db", Type: "worker"}, + }, + }, + }, services: []common.ApplicationComponentStatus{ { Name: "web", @@ -214,6 +226,7 @@ func TestBuildServiceDetails(t *testing.T) { want: []map[string]interface{}{ { "name": "web", + "type": "webservice", "namespace": "default", "cluster": "local", "healthy": true, @@ -222,6 +235,7 @@ func TestBuildServiceDetails(t *testing.T) { }, { "name": "db", + "type": "worker", "namespace": "default", "cluster": "local", "healthy": false, @@ -229,11 +243,66 @@ func TestBuildServiceDetails(t *testing.T) { }, }, }, + { + name: "services with traits", + app: &v1beta1.Application{ + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{ + {Name: "web", Type: "webservice"}, + }, + }, + }, + services: []common.ApplicationComponentStatus{ + { + Name: "web", + Namespace: "default", + Cluster: "local", + Healthy: true, + Message: "Running", + Traits: []common.ApplicationTraitStatus{ + { + Type: "ingress", + Healthy: true, + Message: "Ingress ready", + Details: map[string]string{"host": "example.com"}, + }, + { + Type: "autoscaler", + Healthy: false, + Message: "Scaling", + }, + }, + }, + }, + want: []map[string]interface{}{ + { + "name": "web", + "type": "webservice", + "namespace": "default", + "cluster": "local", + "healthy": true, + "message": "Running", + "traits": []map[string]interface{}{ + { + "type": "ingress", + "healthy": true, + "message": "Ingress ready", + "details": map[string]string{"host": "example.com"}, + }, + { + "type": "autoscaler", + "healthy": false, + "message": "Scaling", + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := buildServiceDetails(tt.services) + got := buildServiceDetails(tt.app, tt.services) assert.Equal(t, tt.want, got) }) } @@ -463,6 +532,12 @@ func TestUpdateMetricsAndLogFunction(t *testing.T) { Namespace: "default", UID: "12345", }, + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{ + {Name: "web", Type: "webservice"}, + {Name: "db", Type: "worker"}, + }, + }, Status: common.AppStatus{ Phase: common.ApplicationRunning, Services: []common.ApplicationComponentStatus{