diff --git a/.gitignore b/.gitignore index 893138b46..97d51584b 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,14 @@ vendor/ .vscode .history +# Debug binaries generated by VS Code/Delve +__debug_bin* +*/__debug_bin* + +# Webhook certificates generated at runtime +k8s-webhook-server/ +options.go.bak + pkg/test/vela config/crd/bases _tmp/ diff --git a/charts/vela-core/README.md b/charts/vela-core/README.md index 00664e1e0..2c89d0688 100644 --- a/charts/vela-core/README.md +++ b/charts/vela-core/README.md @@ -100,6 +100,7 @@ helm install --create-namespace -n vela-system kubevela kubevela/vela-core --wai | `featureGates.disableWorkflowContextConfigMapCache` | disable the workflow context's configmap informer cache | `true` | | `featureGates.enableCueValidation` | enable the strict cue validation for cue required parameter fields | `false` | | `featureGates.enableApplicationStatusMetrics` | enable application status metrics and structured logging | `false` | +| `featureGates.validateResourcesExist` | enable webhook validation to check if resource types referenced in definition templates exist in the cluster | `false` | ### MultiCluster parameters diff --git a/charts/vela-core/templates/admission-webhooks/validatingWebhookConfiguration.yaml b/charts/vela-core/templates/admission-webhooks/validatingWebhookConfiguration.yaml index a37531379..11ad4bbe8 100644 --- a/charts/vela-core/templates/admission-webhooks/validatingWebhookConfiguration.yaml +++ b/charts/vela-core/templates/admission-webhooks/validatingWebhookConfiguration.yaml @@ -126,4 +126,30 @@ webhooks: - UPDATE resources: - policydefinitions + - clientConfig: + caBundle: Cg== + service: + name: {{ template "kubevela.name" . }}-webhook + namespace: {{ .Release.Namespace }} + path: /validating-core-oam-dev-v1beta1-workflowstepdefinitions + {{- if .Values.admissionWebhooks.patch.enabled }} + failurePolicy: Ignore + {{- else }} + failurePolicy: Fail + {{- end }} + name: validating.core.oam-dev.v1beta1.workflowstepdefinitions + sideEffects: None + admissionReviewVersions: + - v1beta1 + - v1 + rules: + - apiGroups: + - core.oam.dev + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - workflowstepdefinitions {{- end -}} diff --git a/charts/vela-core/templates/kubevela-controller.yaml b/charts/vela-core/templates/kubevela-controller.yaml index bcf8a0e1d..bda74980f 100644 --- a/charts/vela-core/templates/kubevela-controller.yaml +++ b/charts/vela-core/templates/kubevela-controller.yaml @@ -313,6 +313,7 @@ spec: - "--feature-gates=DisableWorkflowContextConfigMapCache={{- .Values.featureGates.disableWorkflowContextConfigMapCache | toString -}}" - "--feature-gates=EnableCueValidation={{- .Values.featureGates.enableCueValidation | toString -}}" - "--feature-gates=EnableApplicationStatusMetrics={{- .Values.featureGates.enableApplicationStatusMetrics | toString -}}" + - "--feature-gates=ValidateResourcesExist={{- .Values.featureGates.validateResourcesExist | toString -}}" - "--feature-gates=ValidateDefinitionPermissions={{ .Values.authorization.definitionValidationEnabled | toString -}}" {{ if .Values.authentication.enabled }} {{ if .Values.authentication.withUser }} diff --git a/charts/vela-core/values.yaml b/charts/vela-core/values.yaml index f659035c8..77ee3d38c 100644 --- a/charts/vela-core/values.yaml +++ b/charts/vela-core/values.yaml @@ -125,6 +125,7 @@ optimize: ##@param featureGates.disableWorkflowContextConfigMapCache disable the workflow context's configmap informer cache ##@param featureGates.enableCueValidation enable the strict cue validation for cue required parameter fields ##@param featureGates.enableApplicationStatusMetrics enable application status metrics and structured logging +##@param featureGates.validateResourcesExist enable webhook validation to check if resource types referenced in definition templates exist in the cluster ##@param featureGates: gzipResourceTracker: false @@ -142,6 +143,7 @@ featureGates: disableWorkflowContextConfigMapCache: true enableCueValidation: false enableApplicationStatusMetrics: false + validateResourcesExist: false ## @section MultiCluster parameters diff --git a/docs/WEBHOOK_DEBUGGING.md b/docs/WEBHOOK_DEBUGGING.md new file mode 100644 index 000000000..ad544c384 --- /dev/null +++ b/docs/WEBHOOK_DEBUGGING.md @@ -0,0 +1,191 @@ +# KubeVela Webhook Debugging Guide + +This guide explains how to debug KubeVela webhook validation locally, particularly for the feature that validates ComponentDefinitions, TraitDefinitions, and PolicyDefinitions to ensure they don't reference non-existent CRDs. + +## Overview + +The webhook validation feature checks that CUE templates in definitions only reference Kubernetes resources that exist on the cluster. This prevents runtime errors when non-existent CRDs are referenced. + +## Prerequisites + +- Docker Desktop or similar container runtime +- k3d for local Kubernetes clusters +- VS Code with Go extension +- kubectl configured +- openssl for certificate generation + +## Quick Start + +```bash +# 1. Complete setup (cluster + CRDs + webhook) +make webhook-debug-setup + +# 2. Start VS Code debugger +# Press F5 and select "Debug Webhook Validation" +``` + +## Detailed Setup Steps + +### 1. Environment Setup + +```bash +# Create k3d cluster +make k3d-create + +# Install KubeVela CRDs +make manifests +kubectl apply -f charts/vela-core/crds/ +``` + +### 2. Webhook Certificate Setup + +The webhook requires TLS certificates with proper Subject Alternative Names (SANs) for IP addresses. + +```bash +# Generate certificates and create Kubernetes secret +make webhook-setup +``` + +This creates: +- CA certificate and key +- Server certificate with IP SANs (127.0.0.1, Docker internal IP, local machine IP) +- Kubernetes Secret `webhook-server-cert` in `vela-system` namespace +- ValidatingWebhookConfiguration pointing to local debugger + +### 3. Start Debugger in VS Code + +#### VS Code Launch Configuration + +If you're using VSCode add this configuration to `.vscode/launch.json`: + +```json +{ + "version": "0.2.0", + "configurations": [ + { + "name": "Debug Webhook Validation", + "type": "go", + "request": "launch", + "mode": "debug", + "program": "${workspaceFolder}/cmd/core", + "args": [ + "--log-debug=true", + "--metrics-addr=:8080", + "--enable-leader-election=false", + "--use-webhook=true", + "--webhook-port=9445", + "--webhook-cert-dir=${workspaceFolder}/k8s-webhook-server/serving-certs" + ], + "env": { + "KUBECONFIG": "${env:HOME}/.kube/config", + "POD_NAMESPACE": "vela-system" + }, + "showLog": false, + "console": "integratedTerminal" + } + ] +} +``` + +#### Set Breakpoints + +Recommended breakpoint locations: +- `pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler.go` - ComponentDefinition handler +- `pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler.go` - TraitDefinition handler +- `pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler.go` - PolicyDefinition handler +- `pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler.go` - WorkflowDefinition handler + +#### Launch Debugger + +1. Open VS Code +2. Press `F5` or go to Run → Start Debugging +3. Select **"Debug Webhook Validation"** configuration +4. Wait for webhook server to start (look for message about port 9445) + +The debugger configuration includes: +- `--use-webhook=true` - Enables webhook server +- `--webhook-port=9445` - Port for webhook server +- `--webhook-cert-dir` - Path to certificates +- `POD_NAMESPACE=vela-system` - Required for finding the secret + +## Make Targets Reference + +| Target | Description | +|--------|-------------| +| `make webhook-help` | Show webhook debugging help | +| `make webhook-debug-setup` | Complete setup (cluster + CRDs + webhook) | +| `make k3d-create` | Create k3d cluster | +| `make k3d-delete` | Delete k3d cluster | +| `make webhook-setup` | Setup certificates and webhook configuration | +| `make webhook-clean` | Clean up webhook environment | + +## Troubleshooting + +### Connection Refused Error + +If you get "connection refused" errors: +1. Ensure the debugger is running in VS Code +2. Check that port 9445 is not blocked by firewall +3. Verify the webhook server started (check VS Code console) + +### TLS Certificate Errors + +If you get certificate validation errors: +1. Regenerate certificates: `make webhook-setup` +2. Restart the debugger +3. Ensure IP addresses in certificates match your setup + +### Webhook Not Triggering + +If the webhook doesn't trigger: +1. Check ValidatingWebhookConfiguration: `kubectl get validatingwebhookconfiguration` +2. Verify the webhook URL matches your debugger's IP +3. Check namespace is correct (vela-system) + +### Secret Not Found + +If you see "Wait webhook secret" messages: +1. Ensure the secret exists: `kubectl get secret webhook-server-cert -n vela-system` +2. Recreate if needed: `make webhook-setup` + +## How It Works + +1. **Certificate Generation**: Creates TLS certificates with proper SANs for local IPs +2. **Secret Creation**: Stores certificates in Kubernetes secret +3. **Webhook Configuration**: Creates ValidatingWebhookConfiguration pointing to local debugger +4. **Debugger Startup**: VS Code starts the webhook server on port 9445 +5. **Validation**: When definitions are applied, Kubernetes calls the webhook +6. **Debugging**: Breakpoints allow stepping through validation logic + +## Files and Components + +- **Script**: `hack/debug-webhook-setup.sh` - Main setup script +- **Makefile**: `makefiles/develop.mk` - Make targets for debugging +- **VS Code Config**: `.vscode/launch.json` - Debugger configuration +- **Test Files**: `test/webhook-*.yaml` - Test manifests +- **Validation Logic**: `pkg/webhook/utils/utils.go` - Core validation implementation +- **Handlers**: `pkg/webhook/core.oam.dev/v1beta1/*/validating_handler.go` - Resource handlers + +## Clean Up + +```bash +# Clean up webhook setup +make webhook-clean + +# Delete k3d cluster +make k3d-delete +``` + +## Tips + +1. **Always start the debugger before testing** - The webhook configuration points to your local machine +2. **Use breakpoints wisely** - Too many breakpoints can cause timeouts +3. **Check logs** - VS Code Debug Console shows detailed logs +4. **Test both valid and invalid cases** - Ensures validation works correctly +5. **Keep certificates updated** - Regenerate if IPs change + +## Related Documentation + +- [KubeVela Webhook Implementation](../pkg/webhook/README.md) +- [CUE Template Validation](../pkg/webhook/utils/README.md) +- [Admission Webhooks](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/) \ No newline at end of file diff --git a/e2e/plugin/plugin_test.go b/e2e/plugin/plugin_test.go index 8efe5871e..06c033c36 100644 --- a/e2e/plugin/plugin_test.go +++ b/e2e/plugin/plugin_test.go @@ -425,7 +425,7 @@ spec: } outputs: ingress: { - apiVersion: "networking.k8s.io/v1beta1" + apiVersion: "networking.k8s.io/v1" kind: "Ingress" metadata: name: context.name @@ -436,9 +436,14 @@ spec: paths: [ for k, v in parameter.http { path: k + pathType: "Prefix" backend: { - serviceName: context.name - servicePort: v + service: { + name: context.name + port: { + number: v + } + } } }, ] @@ -617,7 +622,7 @@ spec: --- ## From the trait test-ingress -apiVersion: networking.k8s.io/v1beta1 +apiVersion: networking.k8s.io/v1 kind: Ingress metadata: annotations: {} @@ -637,9 +642,12 @@ spec: http: paths: - backend: - serviceName: express-server - servicePort: 80 + service: + name: express-server + port: + number: 80 path: / + pathType: Prefix --- ` @@ -721,7 +729,7 @@ var livediffResult = `Application (test-vela-app) has been modified(*) - app.oam.dev/component: express-server * Component (express-server) / Trait (test-ingress/ingress) has been removed(-) -- apiVersion: networking.k8s.io/v1beta1 +- apiVersion: networking.k8s.io/v1 - kind: Ingress - metadata: - labels: @@ -739,9 +747,12 @@ var livediffResult = `Application (test-vela-app) has been modified(*) - http: - paths: - - backend: -- serviceName: express-server -- servicePort: 80 +- service: +- name: express-server +- port: +- number: 80 - path: / +- pathType: Prefix * Component (new-express-server) has been added(+) + apiVersion: apps/v1 @@ -796,7 +807,7 @@ var livediffResult = `Application (test-vela-app) has been modified(*) + app.oam.dev/component: new-express-server * Component (new-express-server) / Trait (test-ingress/ingress) has been added(+) -+ apiVersion: networking.k8s.io/v1beta1 ++ apiVersion: networking.k8s.io/v1 + kind: Ingress + metadata: + labels: @@ -814,9 +825,12 @@ var livediffResult = `Application (test-vela-app) has been modified(*) + http: + paths: + - backend: -+ serviceName: new-express-server -+ servicePort: 8080 ++ service: ++ name: new-express-server ++ port: ++ number: 8080 + path: / ++ pathType: Prefix ` var testShowComponentDef = ` diff --git a/hack/debug-webhook-setup.sh b/hack/debug-webhook-setup.sh new file mode 100755 index 000000000..54b069ac3 --- /dev/null +++ b/hack/debug-webhook-setup.sh @@ -0,0 +1,243 @@ +#!/bin/bash +# Webhook debugging setup script for KubeVela +# This script sets up everything needed to debug webhooks locally + +set -e + +# Configuration +CERT_DIR="k8s-webhook-server/serving-certs" +NAMESPACE="vela-system" +SECRET_NAME="webhook-server-cert" +WEBHOOK_CONFIG_NAME="kubevela-vela-core-admission" + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +echo -e "${GREEN}=== KubeVela Webhook Debug Setup ===${NC}" + +# Function to check prerequisites +check_prerequisites() { + echo -e "${YELLOW}Checking prerequisites...${NC}" + + # Check kubectl + if ! command -v kubectl &> /dev/null; then + echo -e "${RED}kubectl is not installed${NC}" + exit 1 + fi + + # Check openssl + if ! command -v openssl &> /dev/null; then + echo -e "${RED}openssl is not installed${NC}" + exit 1 + fi + + # Check cluster connectivity + if ! kubectl cluster-info &> /dev/null; then + echo -e "${RED}Cannot connect to Kubernetes cluster${NC}" + echo "Please ensure your kubeconfig is set up correctly" + exit 1 + fi + + # Wait for cluster to be ready + echo "Waiting for cluster nodes to be ready..." + kubectl wait --for=condition=Ready nodes --all --timeout=60s &> /dev/null || true + + echo -e "${GREEN}Prerequisites check passed${NC}" +} + +# Function to create namespace if not exists +create_namespace() { + echo -e "${YELLOW}Creating namespace ${NAMESPACE}...${NC}" + kubectl create namespace ${NAMESPACE} --dry-run=client -o yaml | kubectl apply -f - + echo -e "${GREEN}Namespace ready${NC}" +} + +# Function to generate certificates +generate_certificates() { + echo -e "${YELLOW}Generating webhook certificates...${NC}" + + # Create directory + mkdir -p ${CERT_DIR} + + # Clean old certificates + rm -f ${CERT_DIR}/* + + # Generate CA private key + openssl genrsa -out ${CERT_DIR}/ca.key 2048 + + # Generate CA certificate + openssl req -x509 -new -nodes -key ${CERT_DIR}/ca.key -days 365 -out ${CERT_DIR}/ca.crt \ + -subj "/CN=webhook-ca" + + # 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 + # 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}') + + # Create certificate config with SANs + cat > /tmp/webhook.conf << EOF +[req] +req_extensions = v3_req +distinguished_name = req_distinguished_name +[req_distinguished_name] +[v3_req] +basicConstraints = CA:FALSE +keyUsage = nonRepudiation, digitalSignature, keyEncipherment +subjectAltName = @alt_names +[alt_names] +DNS.1 = localhost +DNS.2 = vela-webhook.${NAMESPACE}.svc +DNS.3 = vela-webhook.${NAMESPACE}.svc.cluster.local +DNS.4 = *.${NAMESPACE}.svc +DNS.5 = *.${NAMESPACE}.svc.cluster.local +IP.1 = 127.0.0.1 +IP.2 = ${HOST_IP} +IP.3 = ${LOCAL_IP} +EOF + + # 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 + + # Generate server certificate with SANs + openssl x509 -req -in /tmp/server.csr -CA ${CERT_DIR}/ca.crt -CAkey ${CERT_DIR}/ca.key \ + -CAcreateserial -out ${CERT_DIR}/tls.crt -days 365 \ + -extensions v3_req -extfile /tmp/webhook.conf + + echo -e "${GREEN}Certificates generated with IP SANs: 127.0.0.1, ${HOST_IP}, ${LOCAL_IP}${NC}" + + # Clean up temp files + rm -f /tmp/server.csr /tmp/webhook.conf +} + +# Function to create Kubernetes secret +create_k8s_secret() { + echo -e "${YELLOW}Creating Kubernetes secret...${NC}" + + # Delete old secret if exists + kubectl delete secret ${SECRET_NAME} -n ${NAMESPACE} --ignore-not-found + + # Create new secret + kubectl create secret tls ${SECRET_NAME} \ + --cert=${CERT_DIR}/tls.crt \ + --key=${CERT_DIR}/tls.key \ + -n ${NAMESPACE} + + echo -e "${GREEN}Secret ${SECRET_NAME} created in namespace ${NAMESPACE}${NC}" +} + +# Function to create webhook configuration +create_webhook_config() { + echo -e "${YELLOW}Creating webhook configuration...${NC}" + + # Get CA bundle + CA_BUNDLE=$(cat ${CERT_DIR}/ca.crt | base64 | tr -d '\n') + + # Delete old webhook configuration if exists + kubectl delete validatingwebhookconfiguration ${WEBHOOK_CONFIG_NAME} --ignore-not-found + + # Create webhook configuration + cat > /tmp/webhook-config.yaml << EOF +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: ${WEBHOOK_CONFIG_NAME} +webhooks: +- name: componentdefinition.core.oam.dev + clientConfig: + url: https://${HOST_IP}:9445/validating-core-oam-dev-v1beta1-componentdefinitions + caBundle: ${CA_BUNDLE} + rules: + - apiGroups: ["core.oam.dev"] + apiVersions: ["v1beta1"] + resources: ["componentdefinitions"] + operations: ["CREATE", "UPDATE"] + admissionReviewVersions: ["v1", "v1beta1"] + sideEffects: None + failurePolicy: Fail +- name: traitdefinition.core.oam.dev + clientConfig: + url: https://${HOST_IP}:9445/validating-core-oam-dev-v1beta1-traitdefinitions + caBundle: ${CA_BUNDLE} + rules: + - apiGroups: ["core.oam.dev"] + apiVersions: ["v1beta1"] + resources: ["traitdefinitions"] + operations: ["CREATE", "UPDATE"] + admissionReviewVersions: ["v1", "v1beta1"] + sideEffects: None + failurePolicy: Fail +- name: policydefinition.core.oam.dev + clientConfig: + url: https://${HOST_IP}:9445/validating-core-oam-dev-v1beta1-policydefinitions + caBundle: ${CA_BUNDLE} + rules: + - apiGroups: ["core.oam.dev"] + apiVersions: ["v1beta1"] + resources: ["policydefinitions"] + operations: ["CREATE", "UPDATE"] + admissionReviewVersions: ["v1", "v1beta1"] + sideEffects: None + failurePolicy: Fail +- name: workflowstepdefinition.core.oam.dev + clientConfig: + url: https://${HOST_IP}:9445/validating-core-oam-dev-v1beta1-workflowstepdefinitions + caBundle: ${CA_BUNDLE} + rules: + - apiGroups: ["core.oam.dev"] + apiVersions: ["v1beta1"] + resources: ["workflowstepdefinitions"] + operations: ["CREATE", "UPDATE"] + admissionReviewVersions: ["v1", "v1beta1"] + sideEffects: None + failurePolicy: Fail +EOF + + kubectl apply -f /tmp/webhook-config.yaml + rm -f /tmp/webhook-config.yaml + + echo -e "${GREEN}Webhook configuration created${NC}" +} + +# Function to show next steps +show_next_steps() { + echo -e "${GREEN}" + echo "=========================================" + echo "Webhook debugging setup complete!" + echo "=========================================" + echo -e "${NC}" + echo "Next steps:" + echo "1. Open VS Code" + 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 "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 "" + echo -e "${GREEN}The webhook will reject ComponentDefinitions with non-existent CRDs${NC}" +} + +# Main execution +main() { + check_prerequisites + create_namespace + generate_certificates + create_k8s_secret + create_webhook_config + show_next_steps +} + +# Run main function +main "$@" \ No newline at end of file diff --git a/makefiles/develop.mk b/makefiles/develop.mk index f957b8447..585e613ab 100644 --- a/makefiles/develop.mk +++ b/makefiles/develop.mk @@ -30,3 +30,75 @@ core-run: fmt vet manifests .PHONY: gen-cue gen-cue: ./hack/cuegen/cuegen.sh $(DIR) $(FLAGS) + +# ============================================================================== +# Webhook Debug and Development Targets + +K3D_CLUSTER_NAME ?= kubevela-debug +K3D_VERSION ?= v1.31.5 + +## webhook-help: Show webhook debugging help +.PHONY: webhook-help +webhook-help: + @echo "=== KubeVela Webhook Debugging Guide ===" + @echo "" + @echo "Quick Start (recommended):" + @echo " 1. make webhook-debug-setup # Complete setup" + @echo " 2. Start VS Code debugger (F5) with 'Debug Webhook Validation'" + @echo "" + @echo "Individual Commands:" + @echo " make k3d-create # Create k3d cluster" + @echo " make k3d-delete # Delete k3d cluster" + @echo " make webhook-setup # Setup webhook (certs + config)" + @echo " make webhook-clean # Clean up webhook setup" + +## k3d-create: Create a k3d cluster for debugging +.PHONY: k3d-create +k3d-create: + @echo "Creating k3d cluster: $(K3D_CLUSTER_NAME)" + @if k3d cluster list | grep -q "^$(K3D_CLUSTER_NAME)"; then \ + echo "k3d cluster $(K3D_CLUSTER_NAME) already exists"; \ + else \ + k3d cluster create "$(K3D_CLUSTER_NAME)" \ + --servers 1 \ + --agents 1 \ + --wait || (echo "Failed to create k3d cluster" && exit 1); \ + fi + @kubectl config use-context "k3d-$(K3D_CLUSTER_NAME)" + @echo "k3d cluster $(K3D_CLUSTER_NAME) ready" + +## k3d-delete: Delete the k3d cluster +.PHONY: k3d-delete +k3d-delete: + @echo "Deleting k3d cluster: $(K3D_CLUSTER_NAME)" + @k3d cluster delete "$(K3D_CLUSTER_NAME)" || true + +## webhook-setup: Setup webhook certificates and configuration +.PHONY: webhook-setup +webhook-setup: + @echo "Setting up webhook certificates and configuration..." + @chmod +x hack/debug-webhook-setup.sh + @./hack/debug-webhook-setup.sh + +## webhook-debug-setup: Complete webhook debug environment setup +.PHONY: webhook-debug-setup +webhook-debug-setup: + @echo "Setting up complete webhook debug environment..." + @$(MAKE) k3d-create + @echo "Waiting for cluster to be ready..." + @sleep 5 + @kubectl wait --for=condition=Ready nodes --all --timeout=60s || true + @echo "Installing KubeVela CRDs..." + @$(MAKE) manifests + @kubectl apply -f charts/vela-core/crds/ --validate=false + @echo "Setting up webhook..." + @$(MAKE) webhook-setup + +## webhook-clean: Clean up webhook debug environment +.PHONY: webhook-clean +webhook-clean: + @echo "Cleaning up webhook debug environment..." + @rm -rf k8s-webhook-server/ + @kubectl delete secret webhook-server-cert -n vela-system --ignore-not-found + @kubectl delete validatingwebhookconfiguration kubevela-vela-core-admission --ignore-not-found + @echo "Webhook debug environment cleaned" diff --git a/makefiles/e2e.mk b/makefiles/e2e.mk index f3b2957d9..3ae7d544d 100644 --- a/makefiles/e2e.mk +++ b/makefiles/e2e.mk @@ -31,6 +31,7 @@ e2e-setup-core-wo-auth: --set multicluster.clusterGateway.image.repository=ghcr.io/oam-dev/cluster-gateway \ --set admissionWebhooks.patch.image.repository=ghcr.io/oam-dev/kube-webhook-certgen/kube-webhook-certgen \ --set featureGates.enableCueValidation=true \ + --set featureGates.validateResourcesExist=true \ --wait kubevela ./charts/vela-core \ --debug @@ -48,10 +49,11 @@ e2e-setup-core-w-auth: ./charts/vela-core \ --set authentication.enabled=true \ --set authentication.withUser=true \ - --set authentication.groupPattern=* \ + --set authentication.groupPattern='*' \ --set featureGates.zstdResourceTracker=true \ --set featureGates.zstdApplicationRevision=true \ --set featureGates.validateComponentWhenSharding=true \ + --set featureGates.validateResourcesExist=true \ --set multicluster.clusterGateway.enabled=true \ --set multicluster.clusterGateway.image.repository=ghcr.io/oam-dev/cluster-gateway \ --set admissionWebhooks.patch.image.repository=ghcr.io/oam-dev/kube-webhook-certgen/kube-webhook-certgen \ @@ -85,6 +87,32 @@ e2e-test: ginkgo -v ./test/e2e-test @$(OK) tests pass +# Run e2e tests with k3d and webhook validation +.PHONY: e2e-test-local +e2e-test-local: + # Create k3d cluster if needed + @k3d cluster create kubevela-debug --servers 1 --agents 1 || true + # Build and load image + docker build -t vela-core:e2e-test -f Dockerfile . --build-arg=VERSION=e2e-test --build-arg=GITVERSION=test + k3d image import vela-core:e2e-test -c kubevela-debug + # Deploy with Helm + kubectl delete validatingwebhookconfiguration kubevela-vela-core-admission 2>/dev/null || true + helm upgrade --install kubevela ./charts/vela-core \ + --namespace vela-system --create-namespace \ + --set image.repository=vela-core \ + --set image.tag=e2e-test \ + --set image.pullPolicy=IfNotPresent \ + --set admissionWebhooks.enabled=true \ + --set featureGates.enableCueValidation=true \ + --set featureGates.validateResourcesExist=true \ + --set applicationRevisionLimit=5 \ + --set controllerArgs.reSyncPeriod=1m \ + --wait --timeout 3m + # Run tests + ginkgo -v ./test/e2e-test + @$(OK) tests pass + + .PHONY: e2e-addon-test e2e-addon-test: cp bin/vela /tmp/ diff --git a/pkg/appfile/template_test.go b/pkg/appfile/template_test.go index 0708f7dd4..c6485664f 100644 --- a/pkg/appfile/template_test.go +++ b/pkg/appfile/template_test.go @@ -155,7 +155,7 @@ func TestLoadTraitTemplate(t *testing.T) { } outputs: ingress: { - apiVersion: "networking.k8s.io/v1beta1" + apiVersion: "networking.k8s.io/v1" kind: "Ingress" metadata: name: context.name @@ -166,9 +166,14 @@ func TestLoadTraitTemplate(t *testing.T) { paths: [ for k, v in parameter.http { path: k + pathType: "Prefix" backend: { - serviceName: context.name - servicePort: v + service: { + name: context.name + port: { + number: v + } + } } }, ] diff --git a/pkg/cue/definition/template_test.go b/pkg/cue/definition/template_test.go index 15a9f5bf5..dfd5965de 100644 --- a/pkg/cue/definition/template_test.go +++ b/pkg/cue/definition/template_test.go @@ -725,7 +725,7 @@ parameter: { } } outputs: ingress: { - apiVersion: "networking.k8s.io/v1beta1" + apiVersion: "networking.k8s.io/v1" kind: "Ingress" metadata: name: context.name @@ -736,9 +736,14 @@ parameter: { http: { paths: [{ path: parameter.path + pathType: "Prefix" backend: { - serviceName: context.name - servicePort: parameter.exposePort + service: { + name: context.name + port: { + number: parameter.exposePort + } + } } }] } @@ -781,7 +786,7 @@ parameter: { "metadata": map[string]interface{}{"name": "testgame-config"}, "data": map[string]interface{}{"enemies": "enemies-data", "lives": "lives-data"}}, }, "t3service": &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "v1", "kind": "Service", "spec": map[string]interface{}{"ports": []interface{}{map[string]interface{}{"port": int64(1080), "targetPort": int64(443)}}, "selector": map[string]interface{}{"app": "test"}}}}, - "t3ingress": &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "networking.k8s.io/v1beta1", "kind": "Ingress", "labels": map[string]interface{}{"config": "enemies-data"}, "metadata": map[string]interface{}{"name": "test"}, "spec": map[string]interface{}{"rules": []interface{}{map[string]interface{}{"host": "example.com", "http": map[string]interface{}{"paths": []interface{}{map[string]interface{}{"backend": map[string]interface{}{"serviceName": "test", "servicePort": int64(1080)}, "path": "ping"}}}}}}}}, + "t3ingress": &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "networking.k8s.io/v1", "kind": "Ingress", "labels": map[string]interface{}{"config": "enemies-data"}, "metadata": map[string]interface{}{"name": "test"}, "spec": map[string]interface{}{"rules": []interface{}{map[string]interface{}{"host": "example.com", "http": map[string]interface{}{"paths": []interface{}{map[string]interface{}{"path": "ping", "pathType": "Prefix", "backend": map[string]interface{}{"service": map[string]interface{}{"name": "test", "port": map[string]interface{}{"number": int64(1080)}}}}}}}}}}}, }, }, "outputs trait with schema": { diff --git a/pkg/features/controller_features.go b/pkg/features/controller_features.go index 841cca988..16a8518f3 100644 --- a/pkg/features/controller_features.go +++ b/pkg/features/controller_features.go @@ -119,6 +119,10 @@ const ( // EnableApplicationStatusMetrics enable the collection and export of application status metrics and structured logging EnableApplicationStatusMetrics = "EnableApplicationStatusMetrics" + + // ValidateResourcesExist enables webhook validation to check if resource types referenced in + // ComponentDefinition/TraitDefinition/WorkflowStepDefinition/PolicyDefinition CUE templates exist in the cluster + ValidateResourcesExist = "ValidateResourcesExist" ) var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -146,6 +150,7 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ DisableWorkflowContextConfigMapCache: {Default: true, PreRelease: featuregate.Alpha}, EnableCueValidation: {Default: false, PreRelease: featuregate.Beta}, EnableApplicationStatusMetrics: {Default: false, PreRelease: featuregate.Alpha}, + ValidateResourcesExist: {Default: false, PreRelease: featuregate.Alpha}, } func init() { diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go new file mode 100644 index 000000000..73c8d282d --- /dev/null +++ b/pkg/logging/logger.go @@ -0,0 +1,173 @@ +/* +Copyright 2024 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package logging provides structured logging utilities for KubeVela webhooks +// with focus on request traceability and observability. +package logging + +import ( + "context" + "time" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// Structured logging field keys - consistent across all handlers for observability +const ( + // Core traceability fields + FieldRequestID = "requestID" // Unique identifier for request correlation + FieldOperation = "operation" // Webhook operation (CREATE/UPDATE/DELETE) + FieldHandler = "handler" // Handler processing the request + FieldStep = "step" // Current processing step + FieldDuration = "durationMs" // Operation duration in milliseconds + + // Resource identification + FieldName = "name" // Resource name + FieldNamespace = "namespace" // Resource namespace + FieldKind = "kind" // Resource kind + FieldGeneration = "generation" // Resource generation + + // User context + FieldUserName = "user" // User making the request + + // Error tracking + FieldError = "error" // Error indicator + FieldSuccess = "success" // Success indicator +) + +// contextKey for storing values in context +type contextKey struct{ name string } + +var ( + requestIDKey = contextKey{name: "requestID"} + loggerKey = contextKey{name: "logger"} +) + +// Logger wraps logr.Logger with structured logging methods +type Logger struct { + logr.Logger +} + +// WithValues adds key-value pairs to the logger +func (l Logger) WithValues(keysAndValues ...interface{}) Logger { + return Logger{Logger: l.Logger.WithValues(keysAndValues...)} +} + +// New creates a new Logger +func New() Logger { + return Logger{Logger: log.Log} +} + +// WithContext returns a Logger from context or creates a new one +func WithContext(ctx context.Context) Logger { + if logger, ok := ctx.Value(loggerKey).(Logger); ok { + return logger + } + return New() +} + +// IntoContext stores the Logger in context +func (l Logger) IntoContext(ctx context.Context) context.Context { + return context.WithValue(ctx, loggerKey, l) +} + +// WithRequestID stores request ID in context +func WithRequestID(ctx context.Context, requestID string) context.Context { + if requestID == "" { + return ctx + } + return context.WithValue(ctx, requestIDKey, requestID) +} + +// RequestIDFrom retrieves request ID from context +func RequestIDFrom(ctx context.Context) (string, bool) { + id, ok := ctx.Value(requestIDKey).(string) + return id, ok && id != "" +} + +// NewHandlerLogger creates a logger for webhook handlers with full request context +func NewHandlerLogger(ctx context.Context, req admission.Request, handlerName string) Logger { + logger := New() + + // Use admission UID as request ID for correlation + requestID := string(req.UID) + if rid, ok := RequestIDFrom(ctx); ok && rid != "" { + requestID = rid + } + + // Build structured log with essential fields for observability + logger = logger.WithValues( + FieldRequestID, requestID, + FieldHandler, handlerName, + FieldOperation, req.Operation, + FieldKind, req.Kind.Kind, + FieldName, req.Name, + FieldNamespace, req.Namespace, + FieldUserName, req.UserInfo.Username, + ) + + return logger +} + +// Helper methods that return the logger with values added +// These don't log directly, so the actual logging call site is preserved + +// WithStep adds a step field to the logger +func (l Logger) WithStep(step string) Logger { + return l.WithValues(FieldStep, step) +} + +// WithSuccess adds success and duration fields to the logger +func (l Logger) WithSuccess(success bool, startTime ...time.Time) Logger { + logger := l.WithValues(FieldSuccess, success) + if len(startTime) > 0 { + duration := time.Since(startTime[0]) + logger = logger.WithValues(FieldDuration, duration.Milliseconds()) + } + return logger +} + +// WithError adds error context to the logger +func (l Logger) WithError(err error) Logger { + return l.WithValues(FieldError, err.Error(), FieldSuccess, false) +} + +// V returns a logger with verbosity level (0=info, 1=debug, 2=trace) +func (l Logger) V(level int) Logger { + return Logger{Logger: l.Logger.V(level)} +} + +// Debug logs debug message (verbosity 1) +func (l Logger) Debug(msg string, keysAndValues ...interface{}) { + l.Logger.V(1).Info(msg, keysAndValues...) +} + +// Trace logs trace message (verbosity 2) +func (l Logger) Trace(msg string, keysAndValues ...interface{}) { + l.Logger.V(2).Info(msg, keysAndValues...) +} + +// Info logs info message +func (l Logger) Info(msg string, keysAndValues ...interface{}) { + l.Logger.Info(msg, keysAndValues...) +} + +// Error logs error message +func (l Logger) Error(err error, msg string, keysAndValues ...interface{}) { + l.Logger.Error(err, msg, keysAndValues...) +} diff --git a/pkg/webhook/core.oam.dev/v1beta1/application/validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/application/validating_handler.go index cdc278571..b02f65d73 100644 --- a/pkg/webhook/core.oam.dev/v1beta1/application/validating_handler.go +++ b/pkg/webhook/core.oam.dev/v1beta1/application/validating_handler.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "net/http" + "time" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/util/validation/field" @@ -31,6 +32,7 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" controller "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev" + "github.com/oam-dev/kubevela/pkg/logging" "github.com/oam-dev/kubevela/pkg/oam/util" ) @@ -62,35 +64,78 @@ func mergeErrors(errs field.ErrorList) error { // Handle validate Application Spec here func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + startTime := time.Now() + ctx = logging.WithRequestID(ctx, string(req.UID)) + logger := logging.NewHandlerLogger(ctx, req, "ApplicationValidator") + + logger.WithStep("start").Info("Starting admission validation for Application resource", "operation", req.Operation, "applicationName", req.Name, "namespace", req.Namespace) + + // Decode the application app := &v1beta1.Application{} if err := h.Decoder.Decode(req, app); err != nil { - return admission.Errored(http.StatusBadRequest, err) + logger.WithStep("decode").WithError(err).Error(err, "Unable to decode admission request payload into Application object - malformed request") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode: %w (requestUID=%s)", err, req.UID)) } + if req.Namespace != "" { app.Namespace = req.Namespace } + logger = logger.WithValues(logging.FieldGeneration, app.Generation) + + workflowSteps := 0 + if app.Spec.Workflow != nil { + workflowSteps = len(app.Spec.Workflow.Steps) + } + + logger.WithStep("decode").Info("Successfully decoded Application from admission request", + "applicationName", app.Name, + "namespace", app.Namespace, + "componentCount", len(app.Spec.Components), + "policyCount", len(app.Spec.Policies), + "workflowSteps", workflowSteps) + ctx = util.SetNamespaceInCtx(ctx, app.Namespace) + switch req.Operation { case admissionv1.Create: + logger.WithStep("validate-create").Info("Validating Application creation - checking components, policies, and workflow configuration") if allErrs := h.ValidateCreate(ctx, app, req); len(allErrs) > 0 { - // http.StatusUnprocessableEntity will NOT report any error descriptions - // to the client, use generic http.StatusBadRequest instead. - return admission.Errored(http.StatusBadRequest, mergeErrors(allErrs)) + mergedErr := mergeErrors(allErrs) + logger.WithStep("validate-create").WithError(mergedErr).Error(mergedErr, "Application creation validation failed - contains invalid components, policies, or workflow steps", "errorCount", len(allErrs), "applicationName", app.Name) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%w (requestUID=%s)", mergedErr, req.UID)) } + logger.WithStep("validate-create").WithSuccess(true).Info("Application creation validation completed successfully - all components, policies, and workflows are valid", "applicationName", app.Name) + case admissionv1.Update: + logger.WithStep("validate-update").Info("Validating Application update - comparing new configuration with existing state") oldApp := &v1beta1.Application{} if err := h.Decoder.DecodeRaw(req.AdmissionRequest.OldObject, oldApp); err != nil { - return admission.Errored(http.StatusBadRequest, simplifyError(err)) + logger.WithStep("decode-old").WithError(err).Error(err, "Unable to decode previous Application state from admission request - cannot validate update") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%w (requestUID=%s)", simplifyError(err), req.UID)) } + + logger = logger.WithValues("oldGeneration", oldApp.Generation) + if app.ObjectMeta.DeletionTimestamp.IsZero() { if allErrs := h.ValidateUpdate(ctx, app, oldApp, req); len(allErrs) > 0 { - return admission.Errored(http.StatusBadRequest, mergeErrors(allErrs)) + mergedErr := mergeErrors(allErrs) + logger.WithStep("validate-update").WithError(mergedErr).Error(mergedErr, "Application update validation failed - new configuration contains invalid changes", "errorCount", len(allErrs), "applicationName", app.Name, "oldGeneration", oldApp.Generation, "newGeneration", app.Generation) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%w (requestUID=%s)", mergedErr, req.UID)) } + logger.WithStep("validate-update").WithSuccess(true).Info("Application update validation completed successfully - configuration changes are valid", "applicationName", app.Name, "generationChange", fmt.Sprintf("%d->%d", oldApp.Generation, app.Generation)) + } else { + logger.WithStep("skip-validation").Info("Skipping Application validation - resource is being deleted and validation is not required", "reason", "deletion-in-progress", "deletionTimestamp", app.DeletionTimestamp) } + + case admissionv1.Delete: + logger.WithStep("skip-validation").Info("Skipping Application validation - DELETE operations do not require content validation", "reason", "delete-operation-no-validation-needed") + default: - // Do nothing for DELETE and CONNECT + logger.WithStep("skip-validation").Info("Skipping Application validation - operation type is not supported by validator", "operation", req.Operation, "reason", "only CREATE, UPDATE, and DELETE operations are handled") } + + logger.WithStep("complete").WithSuccess(true, startTime).Info("Application admission validation completed successfully - resource will be admitted", "applicationName", req.Name, "operation", req.Operation, "namespace", req.Namespace) return admission.ValidationResponse(true, "") } diff --git a/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler.go new file mode 100644 index 000000000..62bc20f67 --- /dev/null +++ b/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler.go @@ -0,0 +1,171 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package componentdefinition + +import ( + "context" + "fmt" + "net/http" + "time" + + admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/logging" + "github.com/oam-dev/kubevela/pkg/oam" + "github.com/oam-dev/kubevela/pkg/oam/util" + webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" +) + +var componentDefGVR = v1beta1.ComponentDefinitionGVR + +// ValidatingHandler handles validation of component definition +type ValidatingHandler struct { + // Decoder decodes object + Decoder admission.Decoder + Client client.Client +} + +var _ admission.Handler = &ValidatingHandler{} + +// Handle validate ComponentDefinition Spec here +func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + startTime := time.Now() + ctx = logging.WithRequestID(ctx, string(req.UID)) + logger := logging.NewHandlerLogger(ctx, req, "ComponentDefinitionValidator") + + // Using the logger methods directly will show the correct file location + logger.WithStep("start").Info("Starting admission validation for ComponentDefinition resource", "operation", req.Operation, "resourceVersion", req.Kind.Version) + + obj := &v1beta1.ComponentDefinition{} + if req.Resource.String() != componentDefGVR.String() { + err := fmt.Errorf("expect resource to be %s", componentDefGVR) + logger.WithStep("resource-check").WithError(err).Error(err, "Admission request targets unexpected resource type - rejecting request", + "expected", componentDefGVR.String(), + "actual", req.Resource.String(), + "operation", req.Operation) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { + if err := h.Decoder.Decode(req, obj); err != nil { + logger.WithStep("decode").WithError(err).Error(err, "Unable to decode admission request payload into ComponentDefinition object - malformed request") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + // Add definition-specific fields to logger + if obj.Spec.Version != "" { + logger = logger.WithValues("version", obj.Spec.Version) + } + logger.WithStep("decode").Info("Successfully decoded ComponentDefinition from admission request", + "definitionName", obj.Name, + "namespace", obj.Namespace, + "workloadType", obj.Spec.Workload.Type, + "hasSchematic", obj.Spec.Schematic != nil) + + // Validate workload + if err := ValidateWorkload(h.Client.RESTMapper(), obj); err != nil { + logger.WithStep("validate-workload").WithError(err).Error(err, "ComponentDefinition workload configuration is invalid - type and definition must be consistent") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-workload").Info("ComponentDefinition workload configuration validated successfully", "workloadType", obj.Spec.Workload.Type) + + // Validate CUE template + if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { + logger.WithStep("validate-cue").Info("Validating CUE template syntax and semantics for ComponentDefinition schematic") + + if err := webhookutils.ValidateCuexTemplate(ctx, obj.Spec.Schematic.CUE.Template); err != nil { + logger.WithStep("validate-cue").WithError(err).Error(err, "CUE template contains syntax errors or invalid constructs - template compilation failed") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + if err := webhookutils.ValidateOutputResourcesExist(obj.Spec.Schematic.CUE.Template, h.Client.RESTMapper(), obj); err != nil { + logger.WithStep("validate-output-resources").WithError(err).Error(err, "CUE template references output resources that don't exist in cluster - unknown resource types detected") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-cue").WithSuccess(true).Info("CUE template validation completed successfully - template is syntactically correct and all output resources exist") + } + + // Validate semantic version + if obj.Spec.Version != "" { + if err := webhookutils.ValidateSemanticVersion(obj.Spec.Version); err != nil { + logger.WithStep("validate-version").WithError(err).Error(err, "ComponentDefinition version does not follow semantic versioning format (x.y.z)", "version", obj.Spec.Version, "expectedFormat", "x.y.z") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-version").Info("ComponentDefinition version follows semantic versioning format", "version", obj.Spec.Version) + } + + // Validate revision + revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] + if len(revisionName) != 0 { + defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) + if err := webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}); err != nil { + logger.WithStep("validate-revision").WithError(err).Error(err, "ComponentDefinition revision conflicts with existing revision or has invalid format", "revisionName", revisionName, "expectedRevisionName", fmt.Sprintf("%s-v%s", obj.Name, revisionName)) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-revision").Info("ComponentDefinition revision validation completed - no conflicts detected", "revisionName", revisionName) + } + + // Check version conflicts + if err := webhookutils.ValidateMultipleDefVersionsNotPresent(obj.Spec.Version, revisionName, obj.Kind); err != nil { + logger.WithStep("validate-version-conflict").WithError(err).Error(err, "ComponentDefinition has conflicting version specifications - cannot have both spec.version and revision annotation", "specVersion", obj.Spec.Version, "revisionName", revisionName) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + // Log successful completion + logger.WithStep("complete").WithSuccess(true, startTime).Info("ComponentDefinition admission validation completed successfully - resource is valid and will be admitted", "definitionName", obj.Name, "operation", req.Operation) + } else { + logger.WithStep("skip-validation").Info("Skipping ComponentDefinition validation - operation does not require validation", "operation", req.Operation, "reason", "only CREATE and UPDATE operations are validated") + } + return admission.ValidationResponse(true, "") +} + +// RegisterValidatingHandler will register ComponentDefinition validation to webhook +func RegisterValidatingHandler(mgr manager.Manager) { + server := mgr.GetWebhookServer() + server.Register("/validating-core-oam-dev-v1beta1-componentdefinitions", &webhook.Admission{Handler: &ValidatingHandler{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + }}) +} + +// ValidateWorkload validates whether the Workload field is valid +func ValidateWorkload(mapper meta.RESTMapper, cd *v1beta1.ComponentDefinition) error { + + // If the Type and Definition are all empty, it will be rejected. + if cd.Spec.Workload.Type == "" && cd.Spec.Workload.Definition == (common.WorkloadGVK{}) { + return fmt.Errorf("neither the type nor the definition of the workload field in the ComponentDefinition %s can be empty", cd.Name) + } + + // if Type and Definitiondon‘t point to the same workloaddefinition, it will be rejected. + if cd.Spec.Workload.Type != "" && cd.Spec.Workload.Definition != (common.WorkloadGVK{}) { + defRef, err := util.ConvertWorkloadGVK2Definition(mapper, cd.Spec.Workload.Definition) + if err != nil { + return err + } + if defRef.Name != cd.Spec.Workload.Type { + return fmt.Errorf("the type and the definition of the workload field in ComponentDefinition %s should represent the same workload", cd.Name) + } + } + return nil +} diff --git a/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler_test.go similarity index 64% rename from pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler_test.go rename to pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler_test.go index 3e1c95787..4e57758c3 100644 --- a/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler_test.go +++ b/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/component_definition_validating_handler_test.go @@ -30,6 +30,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,6 +40,7 @@ import ( core "github.com/oam-dev/kubevela/apis/core.oam.dev" "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/features" ) var handler ValidatingHandler @@ -153,7 +155,7 @@ var _ = Describe("Test ComponentDefinition validating handler", func() { resp := handler.Handle(context.TODO(), req) Expect(resp.Allowed).Should(BeFalse()) Expect(resp.Result.Reason).Should(Equal(metav1.StatusReason(http.StatusText(http.StatusForbidden)))) - Expect(resp.Result.Message).Should(Equal("neither the type nor the definition of the workload field in the ComponentDefinition wrongCd can be empty")) + Expect(resp.Result.Message).Should(ContainSubstring("neither the type nor the definition of the workload field in the ComponentDefinition wrongCd can be empty")) }) It("Test componentDefinition which type and definition point to different workload type", func() { @@ -176,7 +178,7 @@ var _ = Describe("Test ComponentDefinition validating handler", func() { resp := handler.Handle(context.TODO(), req) Expect(resp.Allowed).Should(BeFalse()) Expect(resp.Result.Reason).Should(Equal(metav1.StatusReason(http.StatusText(http.StatusForbidden)))) - Expect(resp.Result.Message).Should(Equal("the type and the definition of the workload field in ComponentDefinition wrongCd should represent the same workload")) + Expect(resp.Result.Message).Should(ContainSubstring("the type and the definition of the workload field in ComponentDefinition wrongCd should represent the same workload")) }) It("Test cue template validation passed", func() { cd.Spec = v1beta1.ComponentDefinitionSpec{ @@ -401,5 +403,278 @@ var _ = Describe("Test ComponentDefinition validating handler", func() { Expect(resp.Allowed).Should(BeTrue()) }) + It("Test ComponentDefinition with non-existent CRD in outputs should be rejected", func() { + // Enable the ValidateResourcesExist feature gate for this test + originalState := utilfeature.DefaultMutableFeatureGate.Enabled(features.ValidateResourcesExist) + defer utilfeature.DefaultMutableFeatureGate.SetFromMap(map[string]bool{ + string(features.ValidateResourcesExist): originalState, + }) + utilfeature.DefaultMutableFeatureGate.SetFromMap(map[string]bool{ + string(features.ValidateResourcesExist): true, + }) + + templateWithInvalidCRD := ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + invalidResource: { + apiVersion: "custom.io/v1alpha1" + kind: "NonExistentResource" + metadata: name: parameter.name + "-custom" + spec: { + foo: "bar" + } + } +}` + + cd := v1beta1.ComponentDefinition{} + cd.SetGroupVersionKind(v1beta1.ComponentDefinitionGroupVersionKind) + cd.SetName("test-invalid-outputs") + cd.Spec = v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Type: "deployments.apps", + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: templateWithInvalidCRD, + }, + }, + } + cdRaw, _ := json.Marshal(cd) + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: cdRaw}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeFalse()) + Expect(resp.Result.Message).Should(ContainSubstring("resource type not found on cluster")) + }) + + It("Test ComponentDefinition with valid outputs should pass", func() { + templateWithValidOutputs := ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + service: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{ + port: 80 + targetPort: 8080 + }] + } + } + configmap: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: name: parameter.name + "-config" + data: { + config: "test-config" + } + } +}` + + cd := v1beta1.ComponentDefinition{} + cd.SetGroupVersionKind(v1beta1.ComponentDefinitionGroupVersionKind) + cd.SetName("test-valid-outputs") + cd.Spec = v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Type: "deployments.apps", + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: templateWithValidOutputs, + }, + }, + } + cdRaw, _ := json.Marshal(cd) + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: cdRaw}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeTrue()) + }) + + It("Test ComponentDefinition with mixed valid and non-k8s outputs should pass", func() { + templateWithMixedOutputs := ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + service: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{port: 80}] + } + } + customData: { + field1: "value1" + field2: "value2" + nested: { + data: "some-data" + } + } +}` + + cd := v1beta1.ComponentDefinition{} + cd.SetGroupVersionKind(v1beta1.ComponentDefinitionGroupVersionKind) + cd.SetName("test-mixed-outputs") + cd.Spec = v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Type: "deployments.apps", + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: templateWithMixedOutputs, + }, + }, + } + cdRaw, _ := json.Marshal(cd) + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: cdRaw}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeTrue()) + }) + + It("Test ComponentDefinition with empty outputs should pass", func() { + templateWithEmptyOutputs := ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: {}` + + cd := v1beta1.ComponentDefinition{} + cd.SetGroupVersionKind(v1beta1.ComponentDefinitionGroupVersionKind) + cd.SetName("test-empty-outputs") + cd.Spec = v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Type: "deployments.apps", + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: templateWithEmptyOutputs, + }, + }, + } + cdRaw, _ := json.Marshal(cd) + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Resource: reqResource, + Object: runtime.RawExtension{Raw: cdRaw}, + }, + } + resp := handler.Handle(context.TODO(), req) + Expect(resp.Allowed).Should(BeTrue()) + }) + }) }) diff --git a/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler.go deleted file mode 100644 index 8622e21ae..000000000 --- a/pkg/webhook/core.oam.dev/v1beta1/componentdefinition/validating_handler.go +++ /dev/null @@ -1,127 +0,0 @@ -/* - Copyright 2021. The KubeVela Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package componentdefinition - -import ( - "context" - "fmt" - "net/http" - - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/api/meta" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/oam-dev/kubevela/apis/core.oam.dev/common" - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/pkg/oam" - "github.com/oam-dev/kubevela/pkg/oam/util" - webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" -) - -var componentDefGVR = v1beta1.ComponentDefinitionGVR - -// ValidatingHandler handles validation of component definition -type ValidatingHandler struct { - // Decoder decodes object - Decoder admission.Decoder - Client client.Client -} - -var _ admission.Handler = &ValidatingHandler{} - -// Handle validate ComponentDefinition Spec here -func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { - obj := &v1beta1.ComponentDefinition{} - if req.Resource.String() != componentDefGVR.String() { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("expect resource to be %s", componentDefGVR)) - } - - if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { - err := h.Decoder.Decode(req, obj) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - err = ValidateWorkload(h.Client.RESTMapper(), obj) - if err != nil { - return admission.Denied(err.Error()) - } - - // validate cueTemplate - if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { - err = webhookutils.ValidateCuexTemplate(ctx, obj.Spec.Schematic.CUE.Template) - if err != nil { - return admission.Denied(err.Error()) - } - } - - if obj.Spec.Version != "" { - err = webhookutils.ValidateSemanticVersion(obj.Spec.Version) - if err != nil { - return admission.Denied(err.Error()) - } - } - - revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] - if len(revisionName) != 0 { - defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) - err = webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}) - if err != nil { - return admission.Denied(err.Error()) - } - } - - version := obj.Spec.Version - err = webhookutils.ValidateMultipleDefVersionsNotPresent(version, revisionName, obj.Kind) - if err != nil { - return admission.Denied(err.Error()) - } - } - return admission.ValidationResponse(true, "") -} - -// RegisterValidatingHandler will register ComponentDefinition validation to webhook -func RegisterValidatingHandler(mgr manager.Manager) { - server := mgr.GetWebhookServer() - server.Register("/validating-core-oam-dev-v1beta1-componentdefinitions", &webhook.Admission{Handler: &ValidatingHandler{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), - }}) -} - -// ValidateWorkload validates whether the Workload field is valid -func ValidateWorkload(mapper meta.RESTMapper, cd *v1beta1.ComponentDefinition) error { - - // If the Type and Definition are all empty, it will be rejected. - if cd.Spec.Workload.Type == "" && cd.Spec.Workload.Definition == (common.WorkloadGVK{}) { - return fmt.Errorf("neither the type nor the definition of the workload field in the ComponentDefinition %s can be empty", cd.Name) - } - - // if Type and Definitiondon‘t point to the same workloaddefinition, it will be rejected. - if cd.Spec.Workload.Type != "" && cd.Spec.Workload.Definition != (common.WorkloadGVK{}) { - defRef, err := util.ConvertWorkloadGVK2Definition(mapper, cd.Spec.Workload.Definition) - if err != nil { - return err - } - if defRef.Name != cd.Spec.Workload.Type { - return fmt.Errorf("the type and the definition of the workload field in ComponentDefinition %s should represent the same workload", cd.Name) - } - } - return nil -} diff --git a/pkg/webhook/core.oam.dev/v1beta1/policydefinition/policy_definition_validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/policydefinition/policy_definition_validating_handler.go new file mode 100644 index 000000000..5246e5c28 --- /dev/null +++ b/pkg/webhook/core.oam.dev/v1beta1/policydefinition/policy_definition_validating_handler.go @@ -0,0 +1,129 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package policydefinition + +import ( + "context" + "fmt" + "net/http" + "time" + + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/logging" + "github.com/oam-dev/kubevela/pkg/oam" + webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" +) + +var policyDefGVR = v1beta1.PolicyDefinitionGVR + +// ValidatingHandler handles validation of policy definition +type ValidatingHandler struct { + // Decoder decodes object + Decoder admission.Decoder + Client client.Client +} + +var _ admission.Handler = &ValidatingHandler{} + +// Handle validate component definition +func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + startTime := time.Now() + ctx = logging.WithRequestID(ctx, string(req.UID)) + logger := logging.NewHandlerLogger(ctx, req, "PolicyDefinitionValidator") + + logger.WithStep("start").Info("Starting admission validation for PolicyDefinition resource", "operation", req.Operation, "resourceVersion", req.Kind.Version) + + obj := &v1beta1.PolicyDefinition{} + if req.Resource.String() != policyDefGVR.String() { + err := fmt.Errorf("expect resource to be %s", policyDefGVR) + logger.WithStep("resource-check").WithError(err).Error(err, "Admission request targets unexpected resource type - rejecting request", + "expected", policyDefGVR.String(), + "actual", req.Resource.String(), + "operation", req.Operation) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { + if err := h.Decoder.Decode(req, obj); err != nil { + logger.WithStep("decode").WithError(err).Error(err, "Unable to decode admission request payload into PolicyDefinition object - malformed request") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + if obj.Spec.Version != "" { + logger = logger.WithValues("version", obj.Spec.Version) + } + logger.WithStep("decode").Info("Successfully decoded PolicyDefinition from admission request", + "definitionName", obj.Name, + "namespace", obj.Namespace, + "hasSchematic", obj.Spec.Schematic != nil, + "version", obj.Spec.Version) + + if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { + logger.WithStep("validate-cue").Info("Validating CUE template syntax and semantics for PolicyDefinition schematic") + if err := webhookutils.ValidateCueTemplate(obj.Spec.Schematic.CUE.Template); err != nil { + logger.WithStep("validate-cue").WithError(err).Error(err, "CUE template contains syntax errors or invalid constructs - template compilation failed") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + if err := webhookutils.ValidateOutputResourcesExist(obj.Spec.Schematic.CUE.Template, h.Client.RESTMapper(), obj); err != nil { + logger.WithStep("validate-output-resources").WithError(err).Error(err, "CUE template references output resources that don't exist in cluster - unknown resource types detected") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-cue").WithSuccess(true).Info("CUE template validation completed successfully - template is syntactically correct and all output resources exist") + } + + if obj.Spec.Version != "" { + if err := webhookutils.ValidateSemanticVersion(obj.Spec.Version); err != nil { + logger.WithStep("validate-version").WithError(err).Error(err, "PolicyDefinition version does not follow semantic versioning format (x.y.z)", "version", obj.Spec.Version, "expectedFormat", "x.y.z") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-version").Info("PolicyDefinition version follows semantic versioning format", "version", obj.Spec.Version) + } + + revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] + if len(revisionName) != 0 { + defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) + if err := webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}); err != nil { + logger.WithStep("validate-revision").WithError(err).Error(err, "PolicyDefinition revision conflicts with existing revision or has invalid format", "revisionName", revisionName, "expectedRevisionName", fmt.Sprintf("%s-v%s", obj.Name, revisionName)) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-revision").Info("PolicyDefinition revision validation completed - no conflicts detected", "revisionName", revisionName) + } + + if err := webhookutils.ValidateMultipleDefVersionsNotPresent(obj.Spec.Version, revisionName, obj.Kind); err != nil { + logger.WithStep("validate-version-conflict").WithError(err).Error(err, "PolicyDefinition has conflicting version specifications - cannot have both spec.version and revision annotation", "specVersion", obj.Spec.Version, "revisionName", revisionName) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("complete").WithSuccess(true, startTime).Info("PolicyDefinition admission validation completed successfully - resource is valid and will be admitted", "definitionName", obj.Name, "operation", req.Operation) + } else { + logger.WithStep("skip-validation").Info("Skipping PolicyDefinition validation - operation does not require validation", "operation", req.Operation, "reason", "only CREATE and UPDATE operations are validated") + } + return admission.ValidationResponse(true, "") +} + +// RegisterValidatingHandler will register ComponentDefinition validation to webhook +func RegisterValidatingHandler(mgr manager.Manager) { + server := mgr.GetWebhookServer() + server.Register("/validating-core-oam-dev-v1beta1-policydefinitions", &webhook.Admission{Handler: &ValidatingHandler{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + }}) +} diff --git a/pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1beta1/policydefinition/policy_definition_validating_handler_test.go similarity index 100% rename from pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler_test.go rename to pkg/webhook/core.oam.dev/v1beta1/policydefinition/policy_definition_validating_handler_test.go diff --git a/pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler.go deleted file mode 100644 index 464c7e85f..000000000 --- a/pkg/webhook/core.oam.dev/v1beta1/policydefinition/validating_handler.go +++ /dev/null @@ -1,99 +0,0 @@ -/* - Copyright 2021. The KubeVela Authors. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package policydefinition - -import ( - "context" - "fmt" - "net/http" - - admissionv1 "k8s.io/api/admission/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/pkg/oam" - webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" -) - -var policyDefGVR = v1beta1.PolicyDefinitionGVR - -// ValidatingHandler handles validation of policy definition -type ValidatingHandler struct { - // Decoder decodes object - Decoder admission.Decoder - Client client.Client -} - -var _ admission.Handler = &ValidatingHandler{} - -// Handle validate component definition -func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { - obj := &v1beta1.PolicyDefinition{} - if req.Resource.String() != policyDefGVR.String() { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("expect resource to be %s", policyDefGVR)) - } - - if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { - err := h.Decoder.Decode(req, obj) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - // validate cueTemplate - if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { - err = webhookutils.ValidateCueTemplate(obj.Spec.Schematic.CUE.Template) - if err != nil { - return admission.Denied(err.Error()) - } - } - - if obj.Spec.Version != "" { - err = webhookutils.ValidateSemanticVersion(obj.Spec.Version) - if err != nil { - return admission.Denied(err.Error()) - } - } - - revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] - if len(revisionName) != 0 { - defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) - err = webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}) - if err != nil { - return admission.Denied(err.Error()) - } - } - - version := obj.Spec.Version - err = webhookutils.ValidateMultipleDefVersionsNotPresent(version, revisionName, obj.Kind) - if err != nil { - return admission.Denied(err.Error()) - } - } - return admission.ValidationResponse(true, "") -} - -// RegisterValidatingHandler will register ComponentDefinition validation to webhook -func RegisterValidatingHandler(mgr manager.Manager) { - server := mgr.GetWebhookServer() - server.Register("/validating-core-oam-dev-v1beta1-policydefinitions", &webhook.Admission{Handler: &ValidatingHandler{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), - }}) -} diff --git a/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler.go new file mode 100644 index 000000000..105cda25c --- /dev/null +++ b/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler.go @@ -0,0 +1,188 @@ +/* +Copyright 2021 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package traitdefinition + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/pkg/errors" + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/appfile" + controller "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev" + "github.com/oam-dev/kubevela/pkg/logging" + "github.com/oam-dev/kubevela/pkg/oam" + webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" +) + +const ( + errValidateDefRef = "error occurs when validating definition reference" + + failInfoDefRefOmitted = "if definition reference is omitted, patch or output with GVK is required" +) + +var traitDefGVR = v1beta1.TraitDefinitionGVR + +// ValidatingHandler handles validation of trait definition +type ValidatingHandler struct { + Client client.Client + + // Decoder decodes object + Decoder admission.Decoder + // Validators validate objects + Validators []TraitDefValidator +} + +// TraitDefValidator validate trait definition +type TraitDefValidator interface { + Validate(context.Context, v1beta1.TraitDefinition) error +} + +// TraitDefValidatorFn implements TraitDefValidator +type TraitDefValidatorFn func(context.Context, v1beta1.TraitDefinition) error + +// Validate implements TraitDefValidator method +func (fn TraitDefValidatorFn) Validate(ctx context.Context, td v1beta1.TraitDefinition) error { + return fn(ctx, td) +} + +var _ admission.Handler = &ValidatingHandler{} + +// Handle validate trait definition +func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + startTime := time.Now() + ctx = logging.WithRequestID(ctx, string(req.UID)) + logger := logging.NewHandlerLogger(ctx, req, "TraitDefinitionValidator") + + logger.WithStep("start").Info("Starting admission validation for TraitDefinition resource", "operation", req.Operation, "resourceVersion", req.Kind.Version) + + obj := &v1beta1.TraitDefinition{} + if req.Resource.String() != traitDefGVR.String() { + err := fmt.Errorf("expect resource to be %s", traitDefGVR) + logger.WithStep("resource-check").WithError(err).Error(err, "Admission request targets unexpected resource type - rejecting request", + "expected", traitDefGVR.String(), + "actual", req.Resource.String(), + "operation", req.Operation) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { + if err := h.Decoder.Decode(req, obj); err != nil { + logger.WithStep("decode").WithError(err).Error(err, "Unable to decode admission request payload into TraitDefinition object - malformed request") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + if obj.Spec.Version != "" { + logger = logger.WithValues("version", obj.Spec.Version) + } + logger.WithStep("decode").Info("Successfully decoded TraitDefinition from admission request", + "definitionName", obj.Name, + "namespace", obj.Namespace, + "hasReference", len(obj.Spec.Reference.Name) > 0, + "hasSchematic", obj.Spec.Schematic != nil) + + for i, validator := range h.Validators { + if err := validator.Validate(ctx, *obj); err != nil { + logger.WithStep(fmt.Sprintf("validator-%d", i)).WithError(err).Error(err, "TraitDefinition custom validator failed - definition does not meet validation requirements", "validatorIndex", i) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + } + + // validate cueTemplate + if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { + logger.WithStep("validate-cue").Info("Validating CUE template syntax and semantics for TraitDefinition schematic") + if err := webhookutils.ValidateCuexTemplate(ctx, obj.Spec.Schematic.CUE.Template); err != nil { + logger.WithStep("validate-cue").WithError(err).Error(err, "CUE template contains syntax errors or invalid constructs - template compilation failed") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + if err := webhookutils.ValidateOutputResourcesExist(obj.Spec.Schematic.CUE.Template, h.Client.RESTMapper(), obj); err != nil { + logger.WithStep("validate-output-resources").WithError(err).Error(err, "CUE template references output resources that don't exist in cluster - unknown resource types detected") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-cue").WithSuccess(true).Info("CUE template validation completed successfully - template is syntactically correct and all output resources exist") + } + + if obj.Spec.Version != "" { + if err := webhookutils.ValidateSemanticVersion(obj.Spec.Version); err != nil { + logger.WithStep("validate-version").WithError(err).Error(err, "TraitDefinition version does not follow semantic versioning format (x.y.z)", "version", obj.Spec.Version, "expectedFormat", "x.y.z") + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + } + + revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] + if len(revisionName) != 0 { + defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) + if err := webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}); err != nil { + logger.WithStep("validate-revision").WithError(err).Error(err, "TraitDefinition revision conflicts with existing revision or has invalid format", "revisionName", revisionName, "expectedRevisionName", fmt.Sprintf("%s-v%s", obj.Name, revisionName)) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + } + + version := obj.Spec.Version + if err := webhookutils.ValidateMultipleDefVersionsNotPresent(version, revisionName, obj.Kind); err != nil { + logger.WithStep("validate-version-conflict").WithError(err).Error(err, "TraitDefinition has conflicting version specifications - cannot have both spec.version and revision annotation", "specVersion", version, "revisionName", revisionName) + return admission.Denied(fmt.Sprintf("%s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("complete").WithSuccess(true, startTime).Info("TraitDefinition admission validation completed successfully - resource is valid and will be admitted", "definitionName", obj.Name, "operation", req.Operation) + } else { + logger.WithStep("skip-validation").Info("Skipping TraitDefinition validation - operation does not require validation", "operation", req.Operation, "reason", "only CREATE and UPDATE operations are validated") + } + return admission.ValidationResponse(true, "") +} + +// RegisterValidatingHandler will register TraitDefinition validation to webhook +func RegisterValidatingHandler(mgr manager.Manager, _ controller.Args) { + server := mgr.GetWebhookServer() + server.Register("/validating-core-oam-dev-v1beta1-traitdefinitions", &webhook.Admission{Handler: &ValidatingHandler{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + Validators: []TraitDefValidator{ + TraitDefValidatorFn(ValidateDefinitionReference), + // add more validators here + }, + }}) +} + +// ValidateDefinitionReference validates whether the trait definition is valid if +// its `.spec.reference` field is unset. +// It's valid if +// it has at least one output, and all outputs must have GVK +// or it has no output but has a patch +// or it has a patch and outputs, and all outputs must have GVK +// TODO(roywang) currently we only validate whether it contains CUE template. +// Further validation, e.g., output with GVK, valid patch, etc, remains to be done. +func ValidateDefinitionReference(_ context.Context, td v1beta1.TraitDefinition) error { + if len(td.Spec.Reference.Name) > 0 { + return nil + } + capability, err := appfile.ConvertTemplateJSON2Object(td.Name, td.Spec.Extension, td.Spec.Schematic) + if err != nil { + return errors.WithMessage(err, errValidateDefRef) + } + if capability.CueTemplate == "" { + return errors.New(failInfoDefRefOmitted) + + } + return nil +} diff --git a/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler_test.go similarity index 99% rename from pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler_test.go rename to pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler_test.go index 9fdf58999..ec8675e9b 100644 --- a/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler_test.go +++ b/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/trait_definition_validating_handler_test.go @@ -168,7 +168,7 @@ var _ = Describe("Test TraitDefinition validating handler", func() { resp := handler.Handle(context.TODO(), req) Expect(resp.Allowed).Should(BeFalse()) Expect(resp.Result.Reason).Should(Equal(metav1.StatusReason(http.StatusText(http.StatusForbidden)))) - Expect(resp.Result.Message).Should(Equal("mock validator error")) + Expect(resp.Result.Message).Should(ContainSubstring("mock validator error")) }) It("Test cue template validation passed", func() { td.Spec = v1beta1.TraitDefinitionSpec{ diff --git a/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler.go deleted file mode 100644 index a850ae301..000000000 --- a/pkg/webhook/core.oam.dev/v1beta1/traitdefinition/validating_handler.go +++ /dev/null @@ -1,160 +0,0 @@ -/* -Copyright 2021 The KubeVela Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package traitdefinition - -import ( - "context" - "fmt" - "net/http" - - "github.com/pkg/errors" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/pkg/appfile" - controller "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev" - "github.com/oam-dev/kubevela/pkg/oam" - webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" -) - -const ( - errValidateDefRef = "error occurs when validating definition reference" - - failInfoDefRefOmitted = "if definition reference is omitted, patch or output with GVK is required" -) - -var traitDefGVR = v1beta1.TraitDefinitionGVR - -// ValidatingHandler handles validation of trait definition -type ValidatingHandler struct { - Client client.Client - - // Decoder decodes object - Decoder admission.Decoder - // Validators validate objects - Validators []TraitDefValidator -} - -// TraitDefValidator validate trait definition -type TraitDefValidator interface { - Validate(context.Context, v1beta1.TraitDefinition) error -} - -// TraitDefValidatorFn implements TraitDefValidator -type TraitDefValidatorFn func(context.Context, v1beta1.TraitDefinition) error - -// Validate implements TraitDefValidator method -func (fn TraitDefValidatorFn) Validate(ctx context.Context, td v1beta1.TraitDefinition) error { - return fn(ctx, td) -} - -var _ admission.Handler = &ValidatingHandler{} - -// Handle validate trait definition -func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { - obj := &v1beta1.TraitDefinition{} - if req.Resource.String() != traitDefGVR.String() { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("expect resource to be %s", traitDefGVR)) - } - - if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { - err := h.Decoder.Decode(req, obj) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - klog.Info("validating ", " name: ", obj.Name, " operation: ", string(req.Operation)) - for _, validator := range h.Validators { - if err := validator.Validate(ctx, *obj); err != nil { - klog.Info("validation failed ", " name: ", obj.Name, " errMsgi: ", err.Error()) - return admission.Denied(err.Error()) - } - } - - // validate cueTemplate - if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { - err = webhookutils.ValidateCuexTemplate(ctx, obj.Spec.Schematic.CUE.Template) - if err != nil { - return admission.Denied(err.Error()) - } - } - - if obj.Spec.Version != "" { - err = webhookutils.ValidateSemanticVersion(obj.Spec.Version) - if err != nil { - return admission.Denied(err.Error()) - } - } - - revisionName := obj.GetAnnotations()[oam.AnnotationDefinitionRevisionName] - if len(revisionName) != 0 { - defRevName := fmt.Sprintf("%s-v%s", obj.Name, revisionName) - err = webhookutils.ValidateDefinitionRevision(ctx, h.Client, obj, client.ObjectKey{Namespace: obj.Namespace, Name: defRevName}) - if err != nil { - return admission.Denied(err.Error()) - } - } - - version := obj.Spec.Version - err = webhookutils.ValidateMultipleDefVersionsNotPresent(version, revisionName, obj.Kind) - if err != nil { - return admission.Denied(err.Error()) - } - klog.Info("validation passed ", " name: ", obj.Name, " operation: ", string(req.Operation)) - } - return admission.ValidationResponse(true, "") -} - -// RegisterValidatingHandler will register TraitDefinition validation to webhook -func RegisterValidatingHandler(mgr manager.Manager, _ controller.Args) { - server := mgr.GetWebhookServer() - server.Register("/validating-core-oam-dev-v1beta1-traitdefinitions", &webhook.Admission{Handler: &ValidatingHandler{ - Client: mgr.GetClient(), - Decoder: admission.NewDecoder(mgr.GetScheme()), - Validators: []TraitDefValidator{ - TraitDefValidatorFn(ValidateDefinitionReference), - // add more validators here - }, - }}) -} - -// ValidateDefinitionReference validates whether the trait definition is valid if -// its `.spec.reference` field is unset. -// It's valid if -// it has at least one output, and all outputs must have GVK -// or it has no output but has a patch -// or it has a patch and outputs, and all outputs must have GVK -// TODO(roywang) currently we only validate whether it contains CUE template. -// Further validation, e.g., output with GVK, valid patch, etc, remains to be done. -func ValidateDefinitionReference(_ context.Context, td v1beta1.TraitDefinition) error { - if len(td.Spec.Reference.Name) > 0 { - return nil - } - capability, err := appfile.ConvertTemplateJSON2Object(td.Name, td.Spec.Extension, td.Spec.Schematic) - if err != nil { - return errors.WithMessage(err, errValidateDefRef) - } - if capability.CueTemplate == "" { - return errors.New(failInfoDefRefOmitted) - - } - return nil -} diff --git a/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/validating_handler.go deleted file mode 100644 index f430d368f..000000000 --- a/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/validating_handler.go +++ /dev/null @@ -1,89 +0,0 @@ -/* -Copyright 2024 The KubeVela Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package workflowstepdefinition - -import ( - "context" - "fmt" - "net/http" - - admissionv1 "k8s.io/api/admission/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/pkg/oam" - webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" -) - -var workflowStepDefGVR = v1beta1.WorkflowStepDefinitionGVR - -// ValidatingHandler handles validation of workflow step definition -type ValidatingHandler struct { - // Decoder decodes object - Decoder admission.Decoder - Client client.Client -} - -// InjectClient injects the client into the ValidatingHandler -func (h *ValidatingHandler) InjectClient(c client.Client) error { - h.Client = c - return nil -} - -// InjectDecoder injects the decoder into the ValidatingHandler -func (h *ValidatingHandler) InjectDecoder(d admission.Decoder) error { - h.Decoder = d - return nil -} - -// Handle validate WorkflowStepDefinition Spec here -func (h *ValidatingHandler) Handle(_ context.Context, req admission.Request) admission.Response { - obj := &v1beta1.WorkflowStepDefinition{} - if req.Resource.String() != workflowStepDefGVR.String() { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("expect resource to be %s", workflowStepDefGVR)) - } - - if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { - err := h.Decoder.Decode(req, obj) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - - if obj.Spec.Version != "" { - err = webhookutils.ValidateSemanticVersion(obj.Spec.Version) - if err != nil { - return admission.Denied(err.Error()) - } - } - revisionName := obj.Annotations[oam.AnnotationDefinitionRevisionName] - version := obj.Spec.Version - err = webhookutils.ValidateMultipleDefVersionsNotPresent(version, revisionName, obj.Kind) - if err != nil { - return admission.Denied(err.Error()) - } - } - return admission.ValidationResponse(true, "") -} - -// RegisterValidatingHandler will register WorkflowStepDefinition validation to webhook -func RegisterValidatingHandler(mgr manager.Manager) { - server := mgr.GetWebhookServer() - server.Register("/validating-core-oam-dev-v1beta1-workflowstepdefinitions", &webhook.Admission{Handler: &ValidatingHandler{}}) -} diff --git a/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler.go b/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler.go new file mode 100644 index 000000000..e159af12d --- /dev/null +++ b/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler.go @@ -0,0 +1,148 @@ +/* +Copyright 2024 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package workflowstepdefinition provides admission control validation +// for WorkflowStepDefinition resources in KubeVela. +package workflowstepdefinition + +import ( + "context" + "fmt" + "net/http" + "time" + + admissionv1 "k8s.io/api/admission/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/logging" + "github.com/oam-dev/kubevela/pkg/oam" + webhookutils "github.com/oam-dev/kubevela/pkg/webhook/utils" +) + +const ( + // ValidationWebhookPath defines the HTTP path for the validation webhook + ValidationWebhookPath = "/validating-core-oam-dev-v1beta1-workflowstepdefinitions" +) + +var ( + workflowStepDefGVR = v1beta1.WorkflowStepDefinitionGVR +) + +// ValidatingHandler handles validation of WorkflowStepDefinition resources. +type ValidatingHandler struct { + Decoder admission.Decoder + Client client.Client +} + +// InjectClient injects the Kubernetes client into the handler. +func (h *ValidatingHandler) InjectClient(c client.Client) error { + h.Client = c + return nil +} + +// InjectDecoder injects the admission decoder into the handler. +func (h *ValidatingHandler) InjectDecoder(d admission.Decoder) error { + h.Decoder = d + return nil +} + +// Handle validates WorkflowStepDefinition resources during admission control. +func (h *ValidatingHandler) Handle(ctx context.Context, req admission.Request) admission.Response { + startTime := time.Now() + ctx = logging.WithRequestID(ctx, string(req.UID)) + logger := logging.NewHandlerLogger(ctx, req, "WorkflowStepDefinitionValidator") + + logger.WithStep("start").Info("Starting admission validation for WorkflowStepDefinition resource", "operation", req.Operation, "resourceVersion", req.Kind.Version) + + // Validate resource type + if req.Resource.String() != workflowStepDefGVR.String() { + err := fmt.Errorf("expected resource to be %s, got %s", workflowStepDefGVR, req.Resource.String()) + logger.WithStep("resource-check").WithError(err).Error(err, "Admission request targets unexpected resource type - rejecting request", + "expected", workflowStepDefGVR.String(), + "actual", req.Resource.String(), + "operation", req.Operation) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("%s (requestUID=%s)", err.Error(), req.UID)) + } + + // Only validate create and update operations + if req.Operation != admissionv1.Create && req.Operation != admissionv1.Update { + logger.WithStep("skip-validation").Info("Skipping WorkflowStepDefinition validation - operation does not require validation", "operation", req.Operation, "reason", "only CREATE and UPDATE operations are validated") + return admission.ValidationResponse(true, "Operation does not require validation") + } + + // Decode the object + obj := &v1beta1.WorkflowStepDefinition{} + if err := h.Decoder.Decode(req, obj); err != nil { + logger.WithStep("decode").WithError(err).Error(err, "Unable to decode admission request payload into WorkflowStepDefinition object - malformed request") + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode: %s (requestUID=%s)", err.Error(), req.UID)) + } + + if obj.Spec.Version != "" { + logger = logger.WithValues("version", obj.Spec.Version) + } + logger.WithStep("decode").Info("Successfully decoded WorkflowStepDefinition from admission request", + "definitionName", obj.Name, + "namespace", obj.Namespace, + "hasSchematic", obj.Spec.Schematic != nil, + "version", obj.Spec.Version) + + // Validate output resources + if obj.Spec.Schematic != nil && obj.Spec.Schematic.CUE != nil { + logger.WithStep("validate-output-resources").Info("Validating output resources referenced in WorkflowStepDefinition CUE template") + if err := webhookutils.ValidateOutputResourcesExist(obj.Spec.Schematic.CUE.Template, h.Client.RESTMapper(), obj); err != nil { + logger.WithStep("validate-output-resources").WithError(err).Error(err, "CUE template references output resources that don't exist in cluster - unknown resource types detected") + return admission.Denied(fmt.Sprintf("output resource validation failed: %s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-output-resources").WithSuccess(true).Info("Output resources validation completed successfully - all referenced resources exist in cluster") + } + + // Validate semantic version + if obj.Spec.Version != "" { + if err := webhookutils.ValidateSemanticVersion(obj.Spec.Version); err != nil { + logger.WithStep("validate-version").WithError(err).Error(err, "WorkflowStepDefinition version does not follow semantic versioning format (x.y.z)", "version", obj.Spec.Version, "expectedFormat", "x.y.z") + return admission.Denied(fmt.Sprintf("semantic version validation failed: %s (requestUID=%s)", err.Error(), req.UID)) + } + logger.WithStep("validate-version").Info("WorkflowStepDefinition version follows semantic versioning format", "version", obj.Spec.Version) + } + + // Validate version conflicts + revisionName := obj.Annotations[oam.AnnotationDefinitionRevisionName] + if err := webhookutils.ValidateMultipleDefVersionsNotPresent(obj.Spec.Version, revisionName, obj.Kind); err != nil { + logger.WithStep("validate-version-conflict").WithError(err).Error(err, "WorkflowStepDefinition has conflicting version specifications - cannot have both spec.version and revision annotation", "specVersion", obj.Spec.Version, "revisionName", revisionName) + return admission.Denied(fmt.Sprintf("definition version conflict: %s (requestUID=%s)", err.Error(), req.UID)) + } + + logger.WithStep("complete").WithSuccess(true, startTime).Info("WorkflowStepDefinition admission validation completed successfully - resource is valid and will be admitted", "definitionName", obj.Name, "operation", req.Operation) + return admission.ValidationResponse(true, "Validation passed") +} + +// RegisterValidatingHandler registers the WorkflowStepDefinition validation webhook with the manager. +func RegisterValidatingHandler(mgr manager.Manager) { + logger := logging.New() + logger.Info("Registering WorkflowStepDefinition validation webhook", "path", ValidationWebhookPath) + + server := mgr.GetWebhookServer() + server.Register(ValidationWebhookPath, &webhook.Admission{ + Handler: &ValidatingHandler{ + Client: mgr.GetClient(), + Decoder: admission.NewDecoder(mgr.GetScheme()), + }, + }) +} diff --git a/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/validating_handler_test.go b/pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler_test.go similarity index 100% rename from pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/validating_handler_test.go rename to pkg/webhook/core.oam.dev/v1beta1/workflowstepdefinition/workflowstep_validating_handler_test.go diff --git a/pkg/webhook/utils/invalid_resource_check.go b/pkg/webhook/utils/invalid_resource_check.go new file mode 100644 index 000000000..2f2d336c2 --- /dev/null +++ b/pkg/webhook/utils/invalid_resource_check.go @@ -0,0 +1,257 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package utils + +import ( + "fmt" + "strings" + + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/parser" + "cuelang.org/go/cue/token" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/oam-dev/kubevela/pkg/features" +) + +// ExtractResourceInfo extracts apiVersion and kind from CUE template without evaluation +func ExtractResourceInfo(cueTemplate string) ([]ResourceInfo, error) { + file, err := parser.ParseFile("", cueTemplate, parser.ParseComments) + if err != nil { + return nil, fmt.Errorf("failed to parse CUE template: %w", err) + } + + var resources []ResourceInfo + + // Walk through the AST to find output and outputs fields + ast.Walk(file, func(node ast.Node) bool { + if n, ok := node.(*ast.Field); ok { + label := extractLabel(n.Label) + if label == "output" || label == "outputs" { + if label == "output" { + // Extract from single output field + if resource := extractResourceFromStruct(n.Value); resource != nil { + resources = append(resources, *resource) + } + } else if label == "outputs" { + // Extract from outputs field (multiple resources) + resources = append(resources, extractResourcesFromOutputs(n.Value)...) + } + } + } + return true + }, nil) + + return resources, nil +} + +type ResourceInfo struct { + APIVersion string + Kind string + Name string // optional, for better error messages +} + +func extractLabel(label ast.Label) string { + switch l := label.(type) { + case *ast.Ident: + return l.Name + case *ast.BasicLit: + if l.Kind == token.STRING { + // Remove quotes + return strings.Trim(l.Value, `"`) + } + } + return "" +} + +func extractResourceFromStruct(expr ast.Expr) *ResourceInfo { + structLit, ok := expr.(*ast.StructLit) + if !ok { + return nil + } + + resource := &ResourceInfo{} + + for _, elt := range structLit.Elts { + field, ok := elt.(*ast.Field) + if !ok { + continue + } + + label := extractLabel(field.Label) + value := extractStringValue(field.Value) + + switch label { + case "apiVersion": + resource.APIVersion = value + case "kind": + resource.Kind = value + case "metadata": + // Try to extract name from metadata.name + if name := extractNameFromMetadata(field.Value); name != "" { + resource.Name = name + } + } + } + + // Only return if we have both apiVersion and kind + if resource.APIVersion != "" && resource.Kind != "" { + return resource + } + return nil +} + +func extractResourcesFromOutputs(expr ast.Expr) []ResourceInfo { + var resources []ResourceInfo + + structLit, ok := expr.(*ast.StructLit) + if !ok { + return resources + } + + for _, elt := range structLit.Elts { + field, ok := elt.(*ast.Field) + if !ok { + continue + } + + if resource := extractResourceFromStruct(field.Value); resource != nil { + // Use the field label as the resource name if not found in metadata + if resource.Name == "" { + resource.Name = extractLabel(field.Label) + } + resources = append(resources, *resource) + } + } + + return resources +} + +func extractStringValue(expr ast.Expr) string { + if e, ok := expr.(*ast.BasicLit); ok { + if e.Kind == token.STRING { + return strings.Trim(e.Value, `"`) + } + } + return "" +} + +func extractNameFromMetadata(expr ast.Expr) string { + structLit, ok := expr.(*ast.StructLit) + if !ok { + return "" + } + + for _, elt := range structLit.Elts { + field, ok := elt.(*ast.Field) + if !ok { + continue + } + + if extractLabel(field.Label) == "name" { + return extractStringValue(field.Value) + } + } + return "" +} + +// ValidateOutputResourcesExist validates that resources referenced in output/outputs fields exist on the cluster +func ValidateOutputResourcesExist(cueTemplate string, mapper meta.RESTMapper, obj client.Object) error { + // Check if feature gate is enabled FIRST before doing anything + if !utilfeature.DefaultMutableFeatureGate.Enabled(features.ValidateResourcesExist) { + return nil // Skip validation if feature is disabled + } + + // Skip validation for addon definitions + // Addons often bundle CRDs with definitions that reference them + if IsAddonDefinition(obj) { + return nil + } + + // Only extract and validate resources if feature is enabled and not an addon + resources, err := ExtractResourceInfo(cueTemplate) + if err != nil { + return fmt.Errorf("failed to extract resource info: %w", err) + } + + for _, resource := range resources { + gvk := schema.GroupVersionKind{ + Version: resource.APIVersion, + Kind: resource.Kind, + } + + // Parse the apiVersion to get group and version + if strings.Contains(resource.APIVersion, "/") { + parts := strings.SplitN(resource.APIVersion, "/", 2) + gvk.Group = parts[0] + gvk.Version = parts[1] + } else { + // Core API resources (like v1) don't have a group + gvk.Group = "" + gvk.Version = resource.APIVersion + } + + _, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) + if err != nil { + ref := fmt.Sprintf("%s/%s", resource.APIVersion, resource.Kind) + return fmt.Errorf("resource type not found on cluster: %s (%w)", ref, err) + } + } + + return nil +} + +// IsAddonDefinition checks if the object is part of an addon installation +// This is a generic solution that works for ALL addons by checking owner references +func IsAddonDefinition(obj client.Object) bool { + if obj == nil { + return false + } + + // Generic approach: Check if the object has an owner reference to an addon application + // All addon definitions get owner references to their addon application (pattern: addon-{name}) + for _, ownerRef := range obj.GetOwnerReferences() { + if ownerRef.Kind == "Application" && + ownerRef.APIVersion == "core.oam.dev/v1beta1" && + strings.HasPrefix(ownerRef.Name, "addon-") { + return true + } + } + + // Fallback checks for edge cases where owner references might not be set yet + labels := obj.GetLabels() + if labels != nil { + // Check if this is managed by an addon application + appName, hasApp := labels["app.oam.dev/name"] + if hasApp && strings.HasPrefix(appName, "addon-") { + return true + } + } + + // Also check annotations for addon markers + annotations := obj.GetAnnotations() + if annotations != nil { + if addonName, hasAddon := annotations["addons.oam.dev/name"]; hasAddon && addonName != "" { + return true + } + } + + return false +} diff --git a/pkg/webhook/utils/invalid_resource_check_test.go b/pkg/webhook/utils/invalid_resource_check_test.go new file mode 100644 index 000000000..c2f9d9bd7 --- /dev/null +++ b/pkg/webhook/utils/invalid_resource_check_test.go @@ -0,0 +1,307 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package utils + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + utilfeature "k8s.io/apiserver/pkg/util/feature" + + "github.com/oam-dev/kubevela/pkg/features" +) + +// fakeRESTMapper implements meta.RESTMapper for tests (minimal). +type fakeRESTMapper struct { + known map[string]bool // key: group|kind|version +} + +func newFakeRESTMapper(entries ...[3]string) *fakeRESTMapper { + m := &fakeRESTMapper{known: map[string]bool{}} + for _, e := range entries { + m.known[e[0]+"|"+e[1]+"|"+e[2]] = true + } + return m +} + +func (f *fakeRESTMapper) key(gk schema.GroupKind, version string) string { + return gk.Group + "|" + gk.Kind + "|" + version +} + +func (f *fakeRESTMapper) Reset() {} +func (f *fakeRESTMapper) KindFor(resource schema.GroupVersionResource) (schema.GroupVersionKind, error) { + return schema.GroupVersionKind{}, errors.New("not implemented") +} +func (f *fakeRESTMapper) KindsFor(resource schema.GroupVersionResource) ([]schema.GroupVersionKind, error) { + return nil, errors.New("not implemented") +} +func (f *fakeRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { + return schema.GroupVersionResource{}, errors.New("not implemented") +} +func (f *fakeRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { + return nil, errors.New("not implemented") +} +func (f *fakeRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + for _, v := range versions { + if f.known[f.key(gk, v)] { + return &meta.RESTMapping{ + Resource: schema.GroupVersionResource{ + Group: gk.Group, + Version: v, + Resource: "", + }, + GroupVersionKind: schema.GroupVersionKind{ + Group: gk.Group, + Version: v, + Kind: gk.Kind, + }, + }, nil + } + } + return nil, errors.New("no match for kind") +} +func (f *fakeRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { + m, err := f.RESTMapping(gk, versions...) + if err != nil { + return nil, err + } + return []*meta.RESTMapping{m}, nil +} +func (f *fakeRESTMapper) ResourceSingularizer(resource string) (string, error) { + return resource, nil +} + +func TestExtractResourceInfo(t *testing.T) { + tests := map[string]struct { + cue string + want []ResourceInfo + wantErr bool + }{ + "singleOutput": { + cue: ` +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: { name: "my-deploy" } + spec: {} +}`, + want: []ResourceInfo{ + {APIVersion: "apps/v1", Kind: "Deployment", Name: "my-deploy"}, + }, + }, + "multipleOutputs": { + cue: ` +outputs: { + first: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: { name: "cfg" } + } + second: { + apiVersion: "batch/v1" + kind: "Job" + } + third: { kind: "Service" } // missing apiVersion ignored +}`, + want: []ResourceInfo{ + {APIVersion: "v1", Kind: "ConfigMap", Name: "cfg"}, + {APIVersion: "batch/v1", Kind: "Job", Name: "second"}, + }, + }, + "parseError": { + cue: ` +output: { + apiVersion: "v1" + kind: "Pod" +`, + wantErr: true, + }, + "outputWithoutKindIgnored": { + cue: ` +output: { + apiVersion: "v1" + metadata: { name: "x" } +}`, + want: []ResourceInfo{}, + }, + "metadataNameOverridesLabel": { + cue: ` +outputs: { + cfg: { apiVersion: "v1", kind: "ConfigMap", metadata: { name: "real" } } +}`, + want: []ResourceInfo{ + {APIVersion: "v1", Kind: "ConfigMap", Name: "real"}, + }, + }, + "fallbackNameCoreGroup": { + cue: ` +outputs: { + svc: { apiVersion: "v1", kind: "Service" } +}`, + want: []ResourceInfo{ + {APIVersion: "v1", Kind: "Service", Name: "svc"}, + }, + }, + "deepMetadataNameIgnoredFallbackToLabel": { + cue: ` +outputs: { + deep: { apiVersion: "v1", kind: "ConfigMap", metadata: { other: { name: "inner" } } } +}`, + want: []ResourceInfo{ + {APIVersion: "v1", Kind: "ConfigMap", Name: "deep"}, + }, + }, + "nonStructOutputsValueIgnored": { + cue: ` +outputs: [ + { apiVersion: "v1", kind: "ConfigMap" } +]`, + want: []ResourceInfo{}, + }, + "missingAPIVersionAndKindEntriesIgnored": { + cue: ` +outputs: { + a: { apiVersion: "v1" } + b: { kind: "ConfigMap" } + c: { metadata: { name: "x" } } +}`, + want: []ResourceInfo{}, + }, + } + + for name, tc := range tests { + tc := tc + t.Run(name, func(t *testing.T) { + got, err := ExtractResourceInfo(tc.cue) + if tc.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + if len(tc.want) == 0 { + assert.Len(t, got, 0) + } else { + assert.Equal(t, tc.want, got) + } + }) + } +} + +func TestValidateOutputResourcesExist(t *testing.T) { + // Enable the ValidateResourcesExist feature gate for these tests + originalState := utilfeature.DefaultMutableFeatureGate.Enabled(features.ValidateResourcesExist) + defer utilfeature.DefaultMutableFeatureGate.SetFromMap(map[string]bool{ + string(features.ValidateResourcesExist): originalState, + }) + utilfeature.DefaultMutableFeatureGate.SetFromMap(map[string]bool{ + string(features.ValidateResourcesExist): true, + }) + + tests := map[string]struct { + cue string + mapper *fakeRESTMapper + wantErr string + }{ + "singleSuccess": { + cue: ` +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: { name: "my-deploy" } +}`, + mapper: newFakeRESTMapper([3]string{"apps", "Deployment", "v1"}), + }, + "multipleSuccess": { + cue: ` +outputs: { + cm: { apiVersion: "v1", kind: "ConfigMap" } + jobRes: { apiVersion: "batch/v1", kind: "Job" } +}`, + mapper: newFakeRESTMapper( + [3]string{"", "ConfigMap", "v1"}, + [3]string{"batch", "Job", "v1"}, + ), + }, + "missingResource": { + cue: ` +output: { + apiVersion: "apps/v1" + kind: "Deployment" +}`, + mapper: newFakeRESTMapper(), + wantErr: "resource type not found on cluster: apps/v1/Deployment", + }, + "firstOkSecondMissing": { + cue: ` +outputs: { + a: { apiVersion: "v1", kind: "ConfigMap" } + b: { apiVersion: "batch/v1", kind: "Job" } +}`, + mapper: newFakeRESTMapper([3]string{"", "ConfigMap", "v1"}), + wantErr: "resource type not found on cluster: batch/v1/Job", + }, + "ignoreIncompleteEntries": { + cue: ` +outputs: { + a: { apiVersion: "v1" } + b: { kind: "ConfigMap" } + c: { apiVersion: "v1", kind: "ConfigMap" } +}`, + mapper: newFakeRESTMapper([3]string{"", "ConfigMap", "v1"}), + }, + "duplicateResourceTypes": { + cue: ` +outputs: { + one: { apiVersion: "v1", kind: "ConfigMap" } + two: { apiVersion: "v1", kind: "ConfigMap" } +}`, + mapper: newFakeRESTMapper([3]string{"", "ConfigMap", "v1"}), + }, + "malformedApiVersionTrailingSlash": { + cue: ` +outputs: { + bad: { apiVersion: "apps/", kind: "Deployment" } +}`, + mapper: newFakeRESTMapper([3]string{"apps", "Deployment", "v1"}), + wantErr: "resource type not found on cluster: apps//Deployment", + }, + "deepMetadataNameDoesNotAffectValidation": { + cue: ` +outputs: { + deep: { apiVersion: "v1", kind: "ConfigMap", metadata: { other: { name: "x" } } } +}`, + mapper: newFakeRESTMapper([3]string{"", "ConfigMap", "v1"}), + }, + } + + for name, tc := range tests { + tc := tc + t.Run(name, func(t *testing.T) { + // Pass nil object for tests - feature gate is disabled by default + err := ValidateOutputResourcesExist(tc.cue, tc.mapper, nil) + if tc.wantErr != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/workflow/providers/legacy/query/handler_test.go b/pkg/workflow/providers/legacy/query/handler_test.go index 94ffbc7a6..35cc52b6b 100644 --- a/pkg/workflow/providers/legacy/query/handler_test.go +++ b/pkg/workflow/providers/legacy/query/handler_test.go @@ -788,7 +788,7 @@ var _ = Describe("Test Query Provider", func() { }, &networkv1.Ingress{ TypeMeta: metav1.TypeMeta{ - APIVersion: "networking.k8s.io/v1beta1", + APIVersion: "networking.k8s.io/v1", }, ObjectMeta: metav1.ObjectMeta{ Name: "ingress-helm", diff --git a/pkg/workflow/providers/query/handler_test.go b/pkg/workflow/providers/query/handler_test.go index ecc43aa04..bdcc05f80 100644 --- a/pkg/workflow/providers/query/handler_test.go +++ b/pkg/workflow/providers/query/handler_test.go @@ -788,7 +788,7 @@ var _ = Describe("Test Query Provider", func() { }, &networkv1.Ingress{ TypeMeta: metav1.TypeMeta{ - APIVersion: "networking.k8s.io/v1beta1", + APIVersion: "networking.k8s.io/v1", }, ObjectMeta: metav1.ObjectMeta{ Name: "ingress-helm", diff --git a/references/appfile/api/appfile_test.go b/references/appfile/api/appfile_test.go index d1e647a79..074b874ee 100644 --- a/references/appfile/api/appfile_test.go +++ b/references/appfile/api/appfile_test.go @@ -223,7 +223,7 @@ outputs: service: { } outputs: ingress: { - apiVersion: "networking.k8s.io/v1beta1" + apiVersion: "networking.k8s.io/v1" kind: "Ingress" spec: { rules: [{ @@ -232,9 +232,14 @@ outputs: ingress: { paths: [ for k, v in parameter.http { path: k + pathType: "Prefix" backend: { - serviceName: context.name - servicePort: v + service: { + name: context.name + port: { + number: v + } + } } } ] diff --git a/references/cli/top/model/suit_test.go b/references/cli/top/model/suit_test.go index fcc1cec0a..66a7aee54 100644 --- a/references/cli/top/model/suit_test.go +++ b/references/cli/top/model/suit_test.go @@ -120,7 +120,7 @@ var _ = BeforeSuite(func() { Kind: "Ingress", Namespace: "default", Name: "ingress-http", - APIVersion: "networking.k8s.io/v1beta1", + APIVersion: "networking.k8s.io/v1", }, }, { diff --git a/references/cli/velaql_test.go b/references/cli/velaql_test.go index 7eb1c46cd..f605813f6 100644 --- a/references/cli/velaql_test.go +++ b/references/cli/velaql_test.go @@ -117,7 +117,7 @@ var _ = Describe("Test velaQL", func() { Kind: "Ingress", Namespace: "default", Name: "ingress-http", - APIVersion: "networking.k8s.io/v1beta1", + APIVersion: "networking.k8s.io/v1", }, }, { @@ -386,7 +386,7 @@ var _ = Describe("Test velaQL", func() { }, &networkv1.Ingress{ TypeMeta: metav1.TypeMeta{ - APIVersion: "networking.k8s.io/v1beta1", + APIVersion: "networking.k8s.io/v1", }, ObjectMeta: metav1.ObjectMeta{ Name: "ingress-helm", diff --git a/test/e2e-test/definition_output_validation_test.go b/test/e2e-test/definition_output_validation_test.go new file mode 100644 index 000000000..567dbff28 --- /dev/null +++ b/test/e2e-test/definition_output_validation_test.go @@ -0,0 +1,1188 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package controllers_test + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/oam/util" +) + +var _ = Describe("Definition Output Validation E2E tests", func() { + ctx := context.Background() + + var namespace string + var ns corev1.Namespace + + BeforeEach(func() { + namespace = randomNamespaceName("def-output-validation") + ns = corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + + Eventually(func() error { + return k8sClient.Create(ctx, &ns) + }, time.Second*3, time.Microsecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + }) + + AfterEach(func() { + By("Clean up resources after a test") + k8sClient.DeleteAllOf(ctx, &v1beta1.ComponentDefinition{}, client.InNamespace(namespace)) + k8sClient.DeleteAllOf(ctx, &v1beta1.TraitDefinition{}, client.InNamespace(namespace)) + k8sClient.DeleteAllOf(ctx, &v1beta1.PolicyDefinition{}, client.InNamespace(namespace)) + k8sClient.DeleteAllOf(ctx, &v1beta1.WorkflowStepDefinition{}, client.InNamespace(namespace)) + + By(fmt.Sprintf("Delete the entire namespace %s", ns.Name)) + Expect(k8sClient.Delete(ctx, &ns, client.PropagationPolicy(metav1.DeletePropagationForeground))).Should(Succeed()) + }) + + Context("Test validation for outputs with non-existent CRDs", func() { + + It("Should reject ComponentDefinition with non-existent CRD in outputs", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-with-invalid-output", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + nonExistentResource: { + apiVersion: "custom.io/v1alpha1" + kind: "NonExistentResource" + metadata: name: parameter.name + "-custom" + spec: { + foo: "bar" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept ComponentDefinition with only valid resources in outputs", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-with-valid-outputs", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + service: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{ + port: 80 + targetPort: 8080 + }] + } + } + configmap: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: name: parameter.name + "-config" + data: { + config: "some-config" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, componentDef)).Should(Succeed()) + }) + + It("Should reject TraitDefinition with non-existent CRD in outputs", func() { + traitDef := &v1beta1.TraitDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "TraitDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-trait-with-invalid-output", + Namespace: namespace, + }, + Spec: v1beta1.TraitDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + replicas: int +} + +outputs: { + hpa: { + apiVersion: "autoscaling/v2" + kind: "HorizontalPodAutoscaler" + metadata: name: context.name + "-hpa" + spec: { + scaleTargetRef: { + apiVersion: "apps/v1" + kind: "Deployment" + name: context.name + } + minReplicas: parameter.replicas + maxReplicas: parameter.replicas * 2 + } + } + customResource: { + apiVersion: "nonexistent.io/v1" + kind: "NonExistentCRD" + metadata: name: context.name + "-custom" + spec: { + data: "test" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, traitDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept TraitDefinition with valid resources", func() { + traitDef := &v1beta1.TraitDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "TraitDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-trait-with-valid-outputs", + Namespace: namespace, + }, + Spec: v1beta1.TraitDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + replicas: int +} + +patch: { + spec: replicas: parameter.replicas +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, traitDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, traitDef)).Should(Succeed()) + }) + + It("Should reject PolicyDefinition with non-existent CRD in outputs", func() { + policyDef := &v1beta1.PolicyDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "PolicyDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy-with-invalid-output", + Namespace: namespace, + }, + Spec: v1beta1.PolicyDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + namespace: string +} + +output: { + apiVersion: "v1" + kind: "Namespace" + metadata: name: parameter.namespace +} + +outputs: { + invalidResource: { + apiVersion: "custom.io/v1beta1" + kind: "CustomPolicy" + metadata: name: parameter.namespace + "-policy" + spec: { + enabled: true + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, policyDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept PolicyDefinition with valid resources", func() { + policyDef := &v1beta1.PolicyDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "PolicyDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy-with-valid-output", + Namespace: namespace, + }, + Spec: v1beta1.PolicyDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + namespace: string +} + +output: { + apiVersion: "v1" + kind: "Namespace" + metadata: name: parameter.namespace +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, policyDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, policyDef)).Should(Succeed()) + }) + + It("Should handle definitions with mixed valid and non-K8s resources", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-with-mixed-outputs", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + service: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{port: 80}] + } + } + customData: { + field1: "value1" + field2: "value2" + nested: { + data: "some-data" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, componentDef)).Should(Succeed()) + }) + + It("Should handle update operations with validation", func() { + // First create a valid definition + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-update-validation", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: "app" + image: "nginx" + }] + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).NotTo(HaveOccurred()) + + // Try to update with invalid output + Eventually(func() error { + var existing v1beta1.ComponentDefinition + if err := k8sClient.Get(ctx, client.ObjectKey{Name: componentDef.Name, Namespace: namespace}, &existing); err != nil { + return err + } + existing.Spec.Schematic.CUE.Template = ` +parameter: { + name: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: "app" + image: "nginx" + }] + } + } +} + +outputs: { + invalidCRD: { + apiVersion: "nonexistent.io/v1" + kind: "NonExistentResource" + metadata: name: parameter.name + spec: {} + } +}` + return k8sClient.Update(ctx, &existing) + }, time.Second*5, time.Millisecond*500).Should(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, componentDef)).Should(Succeed()) + }) + + It("Should reject ComponentDefinition with multiple invalid CRDs", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-multiple-invalid", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +outputs: { + validService: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{port: 80}] + } + } + invalidCRD1: { + apiVersion: "custom.io/v1alpha1" + kind: "CustomResource" + metadata: name: parameter.name + "-custom1" + spec: {foo: "bar"} + } + invalidCRD2: { + apiVersion: "another.io/v1beta1" + kind: "AnotherResource" + metadata: name: parameter.name + "-custom2" + spec: {enabled: true} + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should handle definitions with complex CUE expressions", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-cue-expressions", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +import "strings" + +parameter: { + name: string + image: string + enableService: bool | *false +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: { + name: strings.ToLower(parameter.name) + labels: { + app: parameter.name + version: "v1" + } + } + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +if parameter.enableService { + outputs: service: { + apiVersion: "v1" + kind: "Service" + metadata: name: parameter.name + "-svc" + spec: { + selector: app: parameter.name + ports: [{port: 80, targetPort: 8080}] + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, componentDef)).Should(Succeed()) + }) + + It("Should reject TraitDefinition with mixed valid and invalid outputs", func() { + traitDef := &v1beta1.TraitDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "TraitDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-trait-mixed-validity", + Namespace: namespace, + }, + Spec: v1beta1.TraitDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + replicas: int | *2 + cpu: string | *"100m" + memory: string | *"128Mi" +} + +outputs: { + // Valid resources + service: { + apiVersion: "v1" + kind: "Service" + metadata: name: context.name + "-svc" + spec: { + selector: app: context.name + ports: [{ + port: 80 + targetPort: 8080 + }] + } + } + configmap: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: name: context.name + "-config" + data: { + cpu: parameter.cpu + memory: parameter.memory + } + } + // Invalid CRD - should cause failure + customMonitor: { + apiVersion: "monitoring.custom.io/v1alpha1" + kind: "ServiceMonitor" + metadata: name: context.name + "-monitor" + spec: { + selector: { + matchLabels: { + app: context.name + } + } + endpoints: [{ + port: "metrics" + interval: "30s" + }] + } + } + // Non-K8s object (should be ignored) + metadata: { + version: "1.0.0" + features: ["monitoring", "scaling"] + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, traitDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept PolicyDefinition with only standard Kubernetes resources", func() { + policyDef := &v1beta1.PolicyDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "PolicyDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-policy-standard-resources", + Namespace: namespace, + }, + Spec: v1beta1.PolicyDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + namespace: string + enabled: bool | *true + labels: {...} | *{} +} + +output: { + apiVersion: "v1" + kind: "Namespace" + metadata: { + name: parameter.namespace + labels: parameter.labels + } +} + +outputs: { + networkPolicy: { + apiVersion: "networking.k8s.io/v1" + kind: "NetworkPolicy" + metadata: { + name: parameter.namespace + "-default-deny" + namespace: parameter.namespace + } + spec: { + podSelector: {} + policyTypes: ["Ingress", "Egress"] + } + } + resourceQuota: { + apiVersion: "v1" + kind: "ResourceQuota" + metadata: { + name: parameter.namespace + "-quota" + namespace: parameter.namespace + } + spec: { + hard: { + "requests.cpu": "4" + "requests.memory": "8Gi" + "limits.cpu": "8" + "limits.memory": "16Gi" + "pods": "10" + } + } + } + limitRange: { + apiVersion: "v1" + kind: "LimitRange" + metadata: { + name: parameter.namespace + "-limits" + namespace: parameter.namespace + } + spec: { + limits: [{ + type: "Container" + default: { + cpu: "100m" + memory: "128Mi" + } + defaultRequest: { + cpu: "50m" + memory: "64Mi" + } + }] + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, policyDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, policyDef)).Should(Succeed()) + }) + + It("Should validate definitions with empty outputs", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-empty-outputs", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + image: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: parameter.name + image: parameter.image + }] + } + } +} + +// Empty outputs should be valid +outputs: {}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, componentDef)).Should(Succeed()) + }) + + It("Should reject WorkflowStepDefinition with non-existent CRD in outputs", func() { + workflowStepDef := &v1beta1.WorkflowStepDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "WorkflowStepDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workflowstep-with-invalid-output", + Namespace: namespace, + }, + Spec: v1beta1.WorkflowStepDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + namespace: string + name: string +} + +output: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: { + name: parameter.name + namespace: parameter.namespace + } + data: { + status: "created" + } +} + +outputs: { + invalidStep: { + apiVersion: "workflow.custom.io/v1alpha1" + kind: "WorkflowStep" + metadata: name: parameter.name + "-step" + spec: { + type: "custom" + enabled: true + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, workflowStepDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept WorkflowStepDefinition with valid resources", func() { + workflowStepDef := &v1beta1.WorkflowStepDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "WorkflowStepDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workflowstep-with-valid-outputs", + Namespace: namespace, + }, + Spec: v1beta1.WorkflowStepDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + namespace: string + name: string + data: {...} +} + +output: { + apiVersion: "v1" + kind: "ConfigMap" + metadata: { + name: parameter.name + namespace: parameter.namespace + } + data: parameter.data +} + +outputs: { + secret: { + apiVersion: "v1" + kind: "Secret" + metadata: { + name: parameter.name + "-secret" + namespace: parameter.namespace + } + type: "Opaque" + data: { + key: "dmFsdWU=" // base64 encoded "value" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, workflowStepDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, workflowStepDef)).Should(Succeed()) + }) + + It("Should reject WorkflowStepDefinition with mixed valid and invalid resources", func() { + workflowStepDef := &v1beta1.WorkflowStepDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "WorkflowStepDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workflowstep-mixed-outputs", + Namespace: namespace, + }, + Spec: v1beta1.WorkflowStepDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string + namespace: string +} + +output: { + apiVersion: "batch/v1" + kind: "Job" + metadata: { + name: parameter.name + namespace: parameter.namespace + } + spec: { + template: { + spec: { + containers: [{ + name: "job" + image: "busybox" + command: ["echo", "hello"] + }] + restartPolicy: "Never" + } + } + } +} + +outputs: { + // Valid resource + cronJob: { + apiVersion: "batch/v1" + kind: "CronJob" + metadata: { + name: parameter.name + "-cron" + namespace: parameter.namespace + } + spec: { + schedule: "*/5 * * * *" + jobTemplate: { + spec: { + template: { + spec: { + containers: [{ + name: "cron" + image: "busybox" + command: ["echo", "cron"] + }] + restartPolicy: "Never" + } + } + } + } + } + } + // Invalid custom resource + customScheduler: { + apiVersion: "scheduler.custom.io/v1beta1" + kind: "CustomScheduler" + metadata: { + name: parameter.name + "-scheduler" + } + spec: { + interval: "5m" + action: "notify" + } + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, workflowStepDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + + It("Should accept WorkflowStepDefinition without outputs", func() { + workflowStepDef := &v1beta1.WorkflowStepDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "WorkflowStepDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-workflowstep-no-outputs", + Namespace: namespace, + }, + Spec: v1beta1.WorkflowStepDefinitionSpec{ + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + message: string + delay: int | *1 +} + +// This workflow step just processes data without creating resources +processedData: { + message: parameter.message + timestamp: "2024-01-01T00:00:00Z" + processed: true +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, workflowStepDef) + Expect(err).NotTo(HaveOccurred()) + + // Clean up + Expect(k8sClient.Delete(ctx, workflowStepDef)).Should(Succeed()) + }) + + It("Should reject ComponentDefinition with invalid apiVersion format", func() { + componentDef := &v1beta1.ComponentDefinition{ + TypeMeta: metav1.TypeMeta{ + Kind: "ComponentDefinition", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-comp-invalid-apiversion", + Namespace: namespace, + }, + Spec: v1beta1.ComponentDefinitionSpec{ + Workload: common.WorkloadTypeDescriptor{ + Definition: common.WorkloadGVK{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + }, + Schematic: &common.Schematic{ + CUE: &common.CUE{ + Template: ` +parameter: { + name: string +} + +output: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: parameter.name + spec: { + selector: matchLabels: app: parameter.name + template: { + metadata: labels: app: parameter.name + spec: containers: [{ + name: "nginx" + image: "nginx:latest" + }] + } + } +} + +outputs: { + invalidResource: { + apiVersion: "invalid-format-no-slash" + kind: "SomeResource" + metadata: name: parameter.name + spec: {} + } +}`, + }, + }, + }, + } + + err := k8sClient.Create(ctx, componentDef) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("resource type not found on cluster")) + }) + }) +}) diff --git a/test/e2e-test/definition_revision_test.go b/test/e2e-test/definition_revision_test.go index 63db5e17b..b437011ea 100644 --- a/test/e2e-test/definition_revision_test.go +++ b/test/e2e-test/definition_revision_test.go @@ -930,7 +930,7 @@ outputs: service: { } outputs: ingress: { - apiVersion: "networking.k8s.io/v1beta1" + apiVersion: "networking.k8s.io/v1" kind: "Ingress" metadata: name: context.name @@ -941,9 +941,14 @@ outputs: ingress: { paths: [ for k, v in parameter.http { path: k + pathType: "Prefix" backend: { - serviceName: context.name - servicePort: v + service: { + name: context.name + port: { + number: v + } + } } }, ]