From 0d28979b2cdedecc01b4382bde2673f38e0f38a3 Mon Sep 17 00:00:00 2001 From: David Rochow Date: Thu, 11 Jun 2026 15:43:22 +0200 Subject: [PATCH 1/2] feat: CCRN architecture hardening & code quality refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Systematic refactoring addressing architectural issues, bugs, and missing test coverage in the CCRN codebase. New packages: - pkg/ccrn/ — Zero-dependency types package (ParsedResource, Match, MatchAll, DefaultDomain). Consumers can import without K8s deps. - pkg/parser/lite/ — Lightweight CCRN/URN parser, no K8s dependencies. Supports both ccrn= and urn:ccrn: formats with explicit templates. Bug fixes: - Fix nil logger panics (discard logger default) - Fix broken error format strings in validator - Fix URN panic on malformed input (bounds checking) - Fix LoadCRDsFromDirectory to use filepath.WalkDir for recursion - Fix CRD regex ^ccrn: .+ to ^ccrn=.+ to match actual format - Fix Dockerfile Go version mismatch (1.23 -> 1.24) - Remove stray import "C" and duplicate SPDX headers Architecture improvements: - Full parser (pkg/parser) wraps lite — single implementation, DRY - KubernetesBackend.ValidateResource uses dry-run create (no orphans) - Webhook no longer creates target resources during admission - RBAC scoped to explicit API groups (no wildcards), delete verb removed - Graceful shutdown with 15s drain on SIGTERM/SIGINT - Readiness gate: /healthz returns 503 until first CRD load succeeds - Deterministic ParsedResource.CCRN() output (alphabetical sort) - Go-idiomatic naming (APIGroup, DefaultURNTemplate) Test coverage (0 -> 111 specs): - pkg/ccrn/ — 29 specs - pkg/parser/ — 8 specs - pkg/parser/lite/ — 30 specs - pkg/validation/ — 24 specs - pkg/webhook/ — 20 specs BREAKING CHANGE: Exported identifier renames (ApiGroup -> APIGroup, DEFAULT_URN_TEMPLATE -> DefaultURNTemplate). KubernetesBackend no longer creates real resources (uses dry-run). RBAC no longer grants delete verb. Signed-off-by: David Rochow --- Dockerfile | 2 +- ccrn-chart/templates/crds/ccrn.yaml | 2 +- ccrn-chart/templates/rbac.yaml | 18 +- cmd/webhook/main.go | 27 +- pkg/apis/parsed_resource.go | 35 +- pkg/apis/types.go | 3 - pkg/apis/validation.go | 2 +- pkg/ccrn/ccrn.go | 150 ++++++ pkg/ccrn/ccrn_suite_test.go | 16 + pkg/ccrn/ccrn_test.go | 149 ++++++ pkg/ccrn/wildcard_test.go | 159 ++++++ pkg/parser/lite/parser.go | 157 ++++++ pkg/parser/lite/parser_suite_test.go | 16 + pkg/parser/lite/parser_test.go | 317 ++++++++++++ pkg/parser/parser.go | 191 ++++--- pkg/parser/parser_test.go | 84 ++++ pkg/validation/filesystem_backend.go | 73 ++- pkg/validation/filesystem_backend_test.go | 37 +- pkg/validation/interface.go | 3 - pkg/validation/kubernetes_backend.go | 24 +- .../testdata/subdir/nested_crd.yaml | 35 ++ pkg/validation/validator.go | 10 +- pkg/webhook/admission_test.go | 464 ++++++++++++++++++ pkg/webhook/webhook.go | 83 +++- pkg/webhook/webhook_test.go | 247 ++++++++++ 25 files changed, 2101 insertions(+), 203 deletions(-) create mode 100644 pkg/ccrn/ccrn.go create mode 100644 pkg/ccrn/ccrn_suite_test.go create mode 100644 pkg/ccrn/ccrn_test.go create mode 100644 pkg/ccrn/wildcard_test.go create mode 100644 pkg/parser/lite/parser.go create mode 100644 pkg/parser/lite/parser_suite_test.go create mode 100644 pkg/parser/lite/parser_test.go create mode 100644 pkg/parser/parser_test.go create mode 100644 pkg/validation/testdata/subdir/nested_crd.yaml create mode 100644 pkg/webhook/admission_test.go create mode 100644 pkg/webhook/webhook_test.go diff --git a/Dockerfile b/Dockerfile index 0405ffb..95bfd92 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.23-alpine AS builder +FROM golang:1.24-alpine AS builder WORKDIR /app diff --git a/ccrn-chart/templates/crds/ccrn.yaml b/ccrn-chart/templates/crds/ccrn.yaml index fbc551d..f6b60fb 100644 --- a/ccrn-chart/templates/crds/ccrn.yaml +++ b/ccrn-chart/templates/crds/ccrn.yaml @@ -29,7 +29,7 @@ spec: properties: ccrn: type: string - pattern: "^ccrn: .+" + pattern: "^ccrn=.+" urn: type: string pattern: "^urn:ccrn:.+" diff --git a/ccrn-chart/templates/rbac.yaml b/ccrn-chart/templates/rbac.yaml index e3d10de..9ec0ab0 100644 --- a/ccrn-chart/templates/rbac.yaml +++ b/ccrn-chart/templates/rbac.yaml @@ -9,18 +9,20 @@ metadata: labels: {{- include "ccrn.labels" . | nindent 4 }} rules: + # CCRN custom resource (validate group) - apiGroups: ["validate.{{ .Values.ccrn.apiGroup }}"] resources: ["ccrns", "ccrns/status"] verbs: ["get", "list", "watch", "create", "update", "patch"] - - apiGroups: ["keystone.openstack.{{ .Values.ccrn.apiGroup }}"] + # Target resource groups (dry-run create for validation) + - apiGroups: + - "vmware.{{ .Values.ccrn.apiGroup }}" + - "opensearch.{{ .Values.ccrn.apiGroup }}" + - "ise.{{ .Values.ccrn.apiGroup }}" + - "keystone.openstack.{{ .Values.ccrn.apiGroup }}" + - "rhel-storage.{{ .Values.ccrn.apiGroup }}" resources: ["*"] - verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] - - apiGroups: ["vault.{{ .Values.ccrn.apiGroup }}"] - resources: ["*"] - verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] - - apiGroups: ["*.{{ .Values.ccrn.apiGroup }}"] - resources: ["*"] - verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] + verbs: ["get", "list", "watch", "create", "update", "patch"] + # CRD access for schema discovery - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] verbs: ["get", "list", "watch"] diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 2243010..dbede50 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -1,16 +1,15 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package main import ( + "context" "flag" "os" "os/signal" "syscall" + "time" "github.com/sirupsen/logrus" @@ -55,12 +54,17 @@ func main() { log.Fatalf("Failed to create webhook server: %v", err) } + // Mark server as ready — NewWebhookServerFromConfig performs the initial + // backend.Refresh() which populates the CRD cache. If it failed critically, + // NewWebhookServerFromConfig would have returned an error above. + server.SetReady() + // Set up signal handling for graceful shutdown stop := make(chan os.Signal, 1) signal.Notify(stop, syscall.SIGINT, syscall.SIGTERM) // Start the webhook server in a goroutine - errCh := make(chan error) + errCh := make(chan error, 1) go func() { errCh <- server.Serve(port, certFile, keyFile) }() @@ -69,7 +73,18 @@ func main() { select { case err := <-errCh: log.Fatalf("Webhook server failed: %v", err) - case <-stop: - log.Info("Received shutdown signal, exiting...") + case sig := <-stop: + log.Infof("Received signal %v, initiating graceful shutdown...", sig) } + + // Graceful shutdown with 15-second timeout + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + if err := server.Shutdown(ctx); err != nil { + log.Errorf("Graceful shutdown failed: %v", err) + os.Exit(1) + } + + log.Info("Server shut down gracefully") } diff --git a/pkg/apis/parsed_resource.go b/pkg/apis/parsed_resource.go index a04542f..b1ec072 100644 --- a/pkg/apis/parsed_resource.go +++ b/pkg/apis/parsed_resource.go @@ -4,7 +4,7 @@ package apis import ( - "fmt" + "sort" "strings" ) @@ -13,7 +13,7 @@ type ParsedResource struct { Format string // "CCRN" or "URN" Fields map[string]string Raw string - UrnTemplate string // URN template used for parsing, if applicable + URNTemplate string // URN template used for parsing, if applicable } // CCRN returns the full CCRN string from the parsed resource @@ -22,20 +22,33 @@ func (p *ParsedResource) CCRN() string { if !exists { return "" } - ccrn := "ccrn=" + ccrnString - for key, value := range p.Fields { + + // Collect non-ccrn keys and sort them for deterministic output + keys := make([]string, 0, len(p.Fields)-1) + for key := range p.Fields { if key != "ccrn" { - ccrn += fmt.Sprintf(", %s=%s", key, value) + keys = append(keys, key) } } - return ccrn + sort.Strings(keys) + + var b strings.Builder + b.WriteString("ccrn=") + b.WriteString(ccrnString) + for _, key := range keys { + b.WriteString(", ") + b.WriteString(key) + b.WriteString("=") + b.WriteString(p.Fields[key]) + } + return b.String() } // URN returns the URN string from the parsed resource using the provided template func (p *ParsedResource) URN(template string) string { if template == "" { - if p.UrnTemplate != "" { - template = p.UrnTemplate + if p.URNTemplate != "" { + template = p.URNTemplate } else { return "" } @@ -77,8 +90,8 @@ func (p *ParsedResource) GetKind() string { return "" } -// ApiGroup returns the group from the parsed CCRN or URN -func (p *ParsedResource) ApiGroup() string { +// APIGroup returns the group from the parsed CCRN or URN +func (p *ParsedResource) APIGroup() string { if ccrn, ok := p.Fields["ccrn"]; ok { resourceParts := strings.SplitN(strings.SplitN(ccrn, "/", 2)[0], ".", 2) if len(resourceParts) < 2 { @@ -89,7 +102,7 @@ func (p *ParsedResource) ApiGroup() string { return "" } -// ApiGroup returns the group from the parsed CCRN or URN +// CCRNName returns the name from the parsed CCRN or URN func (p *ParsedResource) CCRNName() string { if ccrn, ok := p.Fields["ccrn"]; ok { name := strings.SplitN(ccrn, "/", 2)[0] diff --git a/pkg/apis/types.go b/pkg/apis/types.go index 3fe01ee..c438a29 100644 --- a/pkg/apis/types.go +++ b/pkg/apis/types.go @@ -1,9 +1,6 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package apis import ( diff --git a/pkg/apis/validation.go b/pkg/apis/validation.go index e7e7eb7..a2221fd 100644 --- a/pkg/apis/validation.go +++ b/pkg/apis/validation.go @@ -13,7 +13,7 @@ type ValidationBackend interface { GetCRD(ccrnVersion string) (*CRDInfo, error) // ValidateResource validates a resource against its schema - // For KubernetesBackend, this creates an actual resource + // For KubernetesBackend, this uses dry-run create (no resource is persisted) // For FilesystemBackend, this validates against OpenAPI schema ValidateResource(namespace string, parsedCCRN *ParsedResource) error diff --git a/pkg/ccrn/ccrn.go b/pkg/ccrn/ccrn.go new file mode 100644 index 0000000..b95fea1 --- /dev/null +++ b/pkg/ccrn/ccrn.go @@ -0,0 +1,150 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +// Package ccrn provides zero-dependency types and utilities for Common Cloud Resource Names. +// This package only imports from the Go standard library (production code). +package ccrn + +import ( + "sort" + "strings" +) + +// Format constants for resource representations. +const ( + FormatCCRN = "CCRN" + FormatURN = "URN" +) + +// DefaultDomain is the default API group domain for CCRN resources. +const DefaultDomain = "ccrn.cloudoperators.dev" + +// APIGroup is the Kubernetes API group for CCRN resources. +const APIGroup = "ccrn.cloudoperators.dev" + +// ParsedResource represents a parsed CCRN or URN resource with its fields. +type ParsedResource struct { + // Format indicates the source format: FormatCCRN or FormatURN. + Format string + + // Fields contains the key-value pairs of the resource (excluding the ccrn key itself). + Fields map[string]string + + // RawInput is the original input string that was parsed. + RawInput string + + // CCRNKey is the CCRN type key (e.g., "pod.k8s.ccrn.cloudoperators.dev/v1"). + CCRNKey string +} + +// CCRN returns the deterministic CCRN string representation with fields sorted alphabetically. +// Returns empty string if CCRNKey is empty. +func (p ParsedResource) CCRN() string { + if p.CCRNKey == "" { + return "" + } + + var b strings.Builder + b.WriteString("ccrn=") + b.WriteString(p.CCRNKey) + + if len(p.Fields) == 0 { + return b.String() + } + + // Sort field keys alphabetically for deterministic output + keys := make([]string, 0, len(p.Fields)) + for k := range p.Fields { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + b.WriteString(", ") + b.WriteString(k) + b.WriteString("=") + b.WriteString(p.Fields[k]) + } + + return b.String() +} + +// URN returns the URN string representation using the given template. +// Template format: "/{field1}/{field2}/..." where {fieldN} are placeholders for field values. +// Returns empty string if template or CCRNKey is empty. +func (p ParsedResource) URN(template string) string { + if template == "" || p.CCRNKey == "" { + return "" + } + + var b strings.Builder + b.WriteString("urn:ccrn:") + b.WriteString(p.CCRNKey) + + // Parse template segments - template starts with / + // e.g., "/{cluster}/{namespace}/{name}" -> ["", "{cluster}", "{namespace}", "{name}"] + segments := strings.Split(template, "/") + + for _, seg := range segments { + if seg == "" { + continue + } + + b.WriteString("/") + + // Check if segment is a placeholder like {fieldName} + if strings.HasPrefix(seg, "{") && strings.HasSuffix(seg, "}") { + fieldName := seg[1 : len(seg)-1] + if val, ok := p.Fields[fieldName]; ok { + b.WriteString(val) + } else { + // Leave placeholder as-is if field not found + b.WriteString(seg) + } + } else { + b.WriteString(seg) + } + } + + return b.String() +} + +// IsCCRNGroup checks if a group belongs to the given CCRN domain. +// A group belongs to the domain if it equals the domain or ends with ".". +func IsCCRNGroup(group, domain string) bool { + if group == domain { + return true + } + return strings.HasSuffix(group, "."+domain) +} + +// Match performs wildcard matching where "*" matches any single segment value. +// Returns true if pattern equals "*" or pattern equals value exactly. +func Match(pattern, value string) bool { + if pattern == "*" { + return true + } + return pattern == value +} + +// MatchAll matches all fields of a pattern against a resource. +// Fields absent from the pattern are treated as implicit wildcards (always match). +// Fields present in the pattern with value "*" also match any value. +// If a pattern field key is not present in the resource, it does not match +// (unless the pattern value is "*"). +func MatchAll(pattern, resource ParsedResource) bool { + for key, patternValue := range pattern.Fields { + resourceValue, exists := resource.Fields[key] + if !exists { + // Key not in resource: only matches if pattern is wildcard + if patternValue != "*" { + return false + } + continue + } + if !Match(patternValue, resourceValue) { + return false + } + } + return true +} diff --git a/pkg/ccrn/ccrn_suite_test.go b/pkg/ccrn/ccrn_suite_test.go new file mode 100644 index 0000000..d23de3d --- /dev/null +++ b/pkg/ccrn/ccrn_suite_test.go @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package ccrn_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCCRN(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "CCRN Package Suite") +} diff --git a/pkg/ccrn/ccrn_test.go b/pkg/ccrn/ccrn_test.go new file mode 100644 index 0000000..3bc5a1e --- /dev/null +++ b/pkg/ccrn/ccrn_test.go @@ -0,0 +1,149 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package ccrn_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/ccrn" +) + +var _ = Describe("ParsedResource", func() { + Context("struct fields", func() { + It("has Format, Fields, RawInput, and CCRNKey fields", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatCCRN, + Fields: map[string]string{"cluster": "eu-de-1", "namespace": "default"}, + RawInput: "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default", + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + } + Expect(pr.Format).To(Equal(ccrn.FormatCCRN)) + Expect(pr.Fields).To(HaveKeyWithValue("cluster", "eu-de-1")) + Expect(pr.RawInput).To(ContainSubstring("ccrn=")) + Expect(pr.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + }) + }) + + Context("Format constants", func() { + It("defines FormatCCRN and FormatURN", func() { + Expect(ccrn.FormatCCRN).To(Equal("CCRN")) + Expect(ccrn.FormatURN).To(Equal("URN")) + }) + }) + + Context("DefaultDomain constant", func() { + It("equals ccrn.cloudoperators.dev", func() { + Expect(ccrn.DefaultDomain).To(Equal("ccrn.cloudoperators.dev")) + }) + }) + + Context("APIGroup constant", func() { + It("equals ccrn.cloudoperators.dev", func() { + Expect(ccrn.APIGroup).To(Equal("ccrn.cloudoperators.dev")) + }) + }) + + Context("CCRN()", func() { + It("returns deterministic alphabetically-sorted field string", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatCCRN, + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + Fields: map[string]string{ + "namespace": "default", + "cluster": "eu-de-1", + "name": "my-pod", + }, + } + result := pr.CCRN() + Expect(result).To(Equal("ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, name=my-pod, namespace=default")) + }) + + It("returns just the ccrn key when no fields are present", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatCCRN, + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + Fields: map[string]string{}, + } + result := pr.CCRN() + Expect(result).To(Equal("ccrn=pod.k8s.ccrn.cloudoperators.dev/v1")) + }) + + It("returns empty string when CCRNKey is empty", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatCCRN, + Fields: map[string]string{"cluster": "eu-de-1"}, + } + result := pr.CCRN() + Expect(result).To(Equal("")) + }) + }) + + Context("URN()", func() { + It("returns correctly formatted URN string from template", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatURN, + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + "name": "my-pod", + }, + } + template := "/{cluster}/{namespace}/{name}" + result := pr.URN(template) + Expect(result).To(Equal("urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod")) + }) + + It("returns empty string when template is empty", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatURN, + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + Fields: map[string]string{"cluster": "eu-de-1"}, + } + result := pr.URN("") + Expect(result).To(Equal("")) + }) + + It("returns empty string when CCRNKey is empty", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatURN, + Fields: map[string]string{"cluster": "eu-de-1"}, + } + result := pr.URN("/{cluster}") + Expect(result).To(Equal("")) + }) + + It("handles template with missing field values gracefully", func() { + pr := ccrn.ParsedResource{ + Format: ccrn.FormatURN, + CCRNKey: "pod.k8s.ccrn.cloudoperators.dev/v1", + Fields: map[string]string{"cluster": "eu-de-1"}, + } + // namespace not in Fields, so it won't be substituted + template := "/{cluster}/{namespace}" + result := pr.URN(template) + // Should still build the URN but leave the placeholder as-is or empty + Expect(result).To(Equal("urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/eu-de-1/{namespace}")) + }) + }) + + Context("IsCCRNGroup()", func() { + It("returns true when group ends with domain", func() { + Expect(ccrn.IsCCRNGroup("k8s.ccrn.cloudoperators.dev", "ccrn.cloudoperators.dev")).To(BeTrue()) + }) + + It("returns true when group equals domain", func() { + Expect(ccrn.IsCCRNGroup("ccrn.cloudoperators.dev", "ccrn.cloudoperators.dev")).To(BeTrue()) + }) + + It("returns false when group does not match domain", func() { + Expect(ccrn.IsCCRNGroup("apps.example.com", "ccrn.cloudoperators.dev")).To(BeFalse()) + }) + + It("returns false for partial match without dot separator", func() { + Expect(ccrn.IsCCRNGroup("notccrn.cloudoperators.dev", "ccrn.cloudoperators.dev")).To(BeFalse()) + }) + }) +}) diff --git a/pkg/ccrn/wildcard_test.go b/pkg/ccrn/wildcard_test.go new file mode 100644 index 0000000..5ad909b --- /dev/null +++ b/pkg/ccrn/wildcard_test.go @@ -0,0 +1,159 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package ccrn_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/ccrn" +) + +var _ = Describe("Wildcard Matching", func() { + Context("Match()", func() { + It("returns true when pattern is *", func() { + Expect(ccrn.Match("*", "anything")).To(BeTrue()) + }) + + It("returns true when pattern is * and value is empty", func() { + Expect(ccrn.Match("*", "")).To(BeTrue()) + }) + + It("returns true when pattern and value are equal", func() { + Expect(ccrn.Match("foo", "foo")).To(BeTrue()) + }) + + It("returns false when pattern and value differ", func() { + Expect(ccrn.Match("foo", "bar")).To(BeFalse()) + }) + + It("returns true when both are empty", func() { + Expect(ccrn.Match("", "")).To(BeTrue()) + }) + + It("returns false when pattern is non-empty and value is empty", func() { + Expect(ccrn.Match("foo", "")).To(BeFalse()) + }) + + It("returns false when pattern is empty and value is non-empty", func() { + Expect(ccrn.Match("", "bar")).To(BeFalse()) + }) + }) + + Context("MatchAll()", func() { + It("returns true when all pattern fields match resource fields", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + "name": "my-pod", + }, + } + Expect(ccrn.MatchAll(pattern, resource)).To(BeTrue()) + }) + + It("returns false when a pattern field does not match", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "production", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + "name": "my-pod", + }, + } + Expect(ccrn.MatchAll(pattern, resource)).To(BeFalse()) + }) + + It("treats * in pattern as wildcard matching any value", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "*", + "namespace": "default", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "us-west-2", + "namespace": "default", + "name": "my-pod", + }, + } + Expect(ccrn.MatchAll(pattern, resource)).To(BeTrue()) + }) + + It("treats absent fields in pattern as implicit wildcards", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + "name": "my-pod", + }, + } + Expect(ccrn.MatchAll(pattern, resource)).To(BeTrue()) + }) + + It("returns true when pattern has no fields (all wildcards)", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{}, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + }, + } + Expect(ccrn.MatchAll(pattern, resource)).To(BeTrue()) + }) + + It("returns false when pattern field references a key not in resource", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "region": "europe", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + }, + } + // "region" is in pattern but not in resource - this should NOT match + Expect(ccrn.MatchAll(pattern, resource)).To(BeFalse()) + }) + + It("returns true when pattern field references a key not in resource but pattern value is *", func() { + pattern := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "region": "*", + }, + } + resource := ccrn.ParsedResource{ + Fields: map[string]string{ + "cluster": "eu-de-1", + "namespace": "default", + }, + } + // "region" is * in pattern, missing in resource - * matches anything including absent + Expect(ccrn.MatchAll(pattern, resource)).To(BeTrue()) + }) + }) +}) diff --git a/pkg/parser/lite/parser.go b/pkg/parser/lite/parser.go new file mode 100644 index 0000000..1ec10b3 --- /dev/null +++ b/pkg/parser/lite/parser.go @@ -0,0 +1,157 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +// Package lite provides a lightweight CCRN/URN parser with no external dependencies +// beyond the standard library and pkg/ccrn. It never panics on malformed input. +package lite + +import ( + "errors" + "fmt" + "strings" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/ccrn" +) + +// ParseCCRN parses a CCRN-format string into a ParsedResource. +// Expected format: ccrn=./, field1=value1, field2=value2 +func ParseCCRN(input string) (*ccrn.ParsedResource, error) { + trimmed := strings.TrimSpace(input) + if !strings.HasPrefix(trimmed, "ccrn=") { + return nil, errors.New("invalid CCRN format: must start with 'ccrn='") + } + + // Split on commas to get field entries + entries := strings.Split(trimmed, ",") + fields := make(map[string]string) + var ccrnKey string + + for _, entry := range entries { + entry = strings.TrimSpace(entry) + if entry == "" { + continue + } + + parts := strings.SplitN(entry, "=", 2) + if len(parts) != 2 { + return nil, fmt.Errorf("invalid field format: %s (must be key=value)", entry) + } + + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + + // Strip surrounding quotes if present + if len(value) >= 2 && value[0] == '"' && value[len(value)-1] == '"' { + value = value[1 : len(value)-1] + } + + if key == "ccrn" { + ccrnKey = value + } else { + fields[key] = value + } + } + + if ccrnKey == "" { + return nil, errors.New("invalid CCRN format: ccrn key is empty or missing") + } + + return &ccrn.ParsedResource{ + Format: ccrn.FormatCCRN, + Fields: fields, + RawInput: input, + CCRNKey: ccrnKey, + }, nil +} + +// ParseURN parses a URN-format string into a ParsedResource using the provided template. +// Expected URN format: urn:ccrn:.////... +// Template format: /{field1}/{field2}/{field3} where {fieldN} are placeholder names. +// The last template field captures remaining path segments (allows slashes in value). +func ParseURN(input string, template string) (*ccrn.ParsedResource, error) { + trimmed := strings.TrimSpace(input) + if !strings.HasPrefix(trimmed, "urn:ccrn:") { + return nil, errors.New("invalid URN format: must start with 'urn:ccrn:'") + } + + if template == "" { + return nil, errors.New("invalid URN template: template must not be empty") + } + + // Extract the body after the prefix + body := strings.TrimPrefix(trimmed, "urn:ccrn:") + if body == "" { + return nil, errors.New("invalid URN format: empty body after 'urn:ccrn:' prefix") + } + + // Parse template to get field names + // Template format: "/{field1}/{field2}/{field3}" + templateFields := parseTemplateFields(template) + if len(templateFields) == 0 { + return nil, errors.New("invalid URN template: no fields defined") + } + + // The body format is: .////... + // We need to split carefully: the first two slash-separated segments form the CCRNKey + // (kind.group/version), and the rest are field values. + // Use SplitN to limit splits so the last field can contain slashes. + totalSegments := 2 + len(templateFields) // 2 for ccrn key parts + field count + parts := strings.SplitN(body, "/", totalSegments) + + if len(parts) < 2 { + return nil, errors.New("invalid URN format: must contain at least type and version segments") + } + + ccrnKey := parts[0] + "/" + parts[1] + valueParts := parts[2:] + + if len(valueParts) < len(templateFields) { + return nil, fmt.Errorf("URN segment count mismatch: template requires %d field(s), got %d", len(templateFields), len(valueParts)) + } + + fields := make(map[string]string, len(templateFields)) + for i, fieldName := range templateFields { + fields[fieldName] = valueParts[i] + } + + return &ccrn.ParsedResource{ + Format: ccrn.FormatURN, + Fields: fields, + RawInput: input, + CCRNKey: ccrnKey, + }, nil +} + +// Parse auto-detects the format of the input and dispatches to ParseCCRN or ParseURN. +// For CCRN format, the template parameter is ignored. +// For URN format, the template parameter is required. +func Parse(input string, template string) (*ccrn.ParsedResource, error) { + trimmed := strings.TrimSpace(input) + if strings.HasPrefix(trimmed, "ccrn=") { + return ParseCCRN(input) + } + if strings.HasPrefix(trimmed, "urn:ccrn:") { + return ParseURN(input, template) + } + return nil, errors.New("unknown format: input must start with 'ccrn=' or 'urn:ccrn:'") +} + +// parseTemplateFields extracts field names from a template like "/{cluster}/{namespace}/{name}". +// Returns the ordered list of field names. +func parseTemplateFields(template string) []string { + segments := strings.Split(template, "/") + var fields []string + for _, seg := range segments { + seg = strings.TrimSpace(seg) + if seg == "" { + continue + } + if strings.HasPrefix(seg, "{") && strings.HasSuffix(seg, "}") { + fieldName := seg[1 : len(seg)-1] + if fieldName != "" { + fields = append(fields, fieldName) + } + } + } + return fields +} diff --git a/pkg/parser/lite/parser_suite_test.go b/pkg/parser/lite/parser_suite_test.go new file mode 100644 index 0000000..b595a87 --- /dev/null +++ b/pkg/parser/lite/parser_suite_test.go @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package lite_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestLiteParser(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Parser Lite Suite") +} diff --git a/pkg/parser/lite/parser_test.go b/pkg/parser/lite/parser_test.go new file mode 100644 index 0000000..54182d5 --- /dev/null +++ b/pkg/parser/lite/parser_test.go @@ -0,0 +1,317 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package lite_test + +import ( + "github.com/cloudoperators/common-cloud-resource-names/pkg/ccrn" + "github.com/cloudoperators/common-cloud-resource-names/pkg/parser/lite" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ParseCCRN", func() { + Context("valid inputs", func() { + It("parses a simple CCRN with multiple fields", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod" + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.Format).To(Equal(ccrn.FormatCCRN)) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("cluster", "eu-de-1")) + Expect(result.Fields).To(HaveKeyWithValue("namespace", "default")) + Expect(result.Fields).To(HaveKeyWithValue("name", "my-pod")) + Expect(result.RawInput).To(Equal(input)) + }) + + It("parses a CCRN with a single field", func() { + input := "ccrn=bucket.s3.ccrn.cloudoperators.dev/v1, name=my-bucket" + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.CCRNKey).To(Equal("bucket.s3.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveLen(1)) + Expect(result.Fields).To(HaveKeyWithValue("name", "my-bucket")) + }) + + It("parses a CCRN with no extra fields (only ccrn key)", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1" + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(BeEmpty()) + }) + + It("handles embedded = in values", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, label=key=value, name=test" + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Fields).To(HaveKeyWithValue("label", "key=value")) + Expect(result.Fields).To(HaveKeyWithValue("name", "test")) + }) + + It("handles extra commas gracefully", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, name=test,," + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("name", "test")) + }) + + It("handles extra spaces gracefully", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1 , name = test , cluster = eu " + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("name", "test")) + Expect(result.Fields).To(HaveKeyWithValue("cluster", "eu")) + }) + + It("handles quoted values by stripping quotes", func() { + input := `ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, name="my-pod"` + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Fields).To(HaveKeyWithValue("name", "my-pod")) + }) + + It("handles wildcard values", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=*, namespace=*, name=*" + result, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Fields).To(HaveKeyWithValue("cluster", "*")) + Expect(result.Fields).To(HaveKeyWithValue("namespace", "*")) + Expect(result.Fields).To(HaveKeyWithValue("name", "*")) + }) + }) + + Context("invalid inputs", func() { + It("returns error for empty string", func() { + _, err := lite.ParseCCRN("") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must start with 'ccrn='")) + }) + + It("returns error for missing ccrn= prefix", func() { + _, err := lite.ParseCCRN("urn:ccrn:something") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must start with 'ccrn='")) + }) + + It("returns error for whitespace-only input", func() { + _, err := lite.ParseCCRN(" ") + Expect(err).To(HaveOccurred()) + }) + + It("returns error for ccrn= with no value", func() { + _, err := lite.ParseCCRN("ccrn=") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("ccrn key")) + }) + + It("returns error for field without value", func() { + _, err := lite.ParseCCRN("ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, badfield") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("key=value")) + }) + + It("never panics on malformed input", func() { + inputs := []string{ + "", + "ccrn=", + "ccrn", + "ccrn=,,,", + "ccrn=x, =y", + string(make([]byte, 10000)), + } + for _, input := range inputs { + Expect(func() { + lite.ParseCCRN(input) //nolint:errcheck + }).ToNot(Panic()) + } + }) + }) +}) + +var _ = Describe("ParseURN", func() { + Context("valid inputs", func() { + It("parses a URN with explicit template", func() { + input := "urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod" + template := "/{cluster}/{namespace}/{name}" + result, err := lite.ParseURN(input, template) + Expect(err).ToNot(HaveOccurred()) + Expect(result).ToNot(BeNil()) + Expect(result.Format).To(Equal(ccrn.FormatURN)) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("cluster", "eu-de-1")) + Expect(result.Fields).To(HaveKeyWithValue("namespace", "default")) + Expect(result.Fields).To(HaveKeyWithValue("name", "my-pod")) + Expect(result.RawInput).To(Equal(input)) + }) + + It("parses a URN with a single-field template", func() { + input := "urn:ccrn:bucket.s3.ccrn.cloudoperators.dev/v1/my-bucket" + template := "/{name}" + result, err := lite.ParseURN(input, template) + Expect(err).ToNot(HaveOccurred()) + Expect(result.CCRNKey).To(Equal("bucket.s3.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("name", "my-bucket")) + }) + + It("handles last field with slashes (path-like values)", func() { + input := "urn:ccrn:object.s3.ccrn.cloudoperators.dev/v1/my-bucket/path/to/file.txt" + template := "/{bucket}/{path}" + result, err := lite.ParseURN(input, template) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Fields).To(HaveKeyWithValue("bucket", "my-bucket")) + Expect(result.Fields).To(HaveKeyWithValue("path", "path/to/file.txt")) + }) + }) + + Context("invalid inputs", func() { + It("returns error for empty string", func() { + _, err := lite.ParseURN("", "/{name}") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must start with 'urn:ccrn:'")) + }) + + It("returns error for wrong prefix", func() { + _, err := lite.ParseURN("ccrn=something", "/{name}") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must start with 'urn:ccrn:'")) + }) + + It("returns error for empty template", func() { + _, err := lite.ParseURN("urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/foo", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("template")) + }) + + It("returns error for URN with insufficient segments", func() { + input := "urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1" + template := "/{cluster}/{namespace}/{name}" + _, err := lite.ParseURN(input, template) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("segment")) + }) + + It("returns error for URN with missing type/version", func() { + _, err := lite.ParseURN("urn:ccrn:podonly", "/{name}") + Expect(err).To(HaveOccurred()) + }) + + It("never panics on malformed input", func() { + inputs := []string{ + "", + "urn:ccrn:", + "urn:ccrn:a", + "urn:ccrn:a/", + "urn:ccrn:///", + string(make([]byte, 10000)), + } + templates := []string{"", "/{name}", "/{a}/{b}/{c}"} + for _, input := range inputs { + for _, tmpl := range templates { + Expect(func() { + lite.ParseURN(input, tmpl) //nolint:errcheck + }).ToNot(Panic()) + } + } + }) + }) +}) + +var _ = Describe("Parse", func() { + Context("auto-detection", func() { + It("dispatches CCRN format correctly", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, name=test" + result, err := lite.Parse(input, "") + Expect(err).ToNot(HaveOccurred()) + Expect(result.Format).To(Equal(ccrn.FormatCCRN)) + Expect(result.CCRNKey).To(Equal("pod.k8s.ccrn.cloudoperators.dev/v1")) + Expect(result.Fields).To(HaveKeyWithValue("name", "test")) + }) + + It("dispatches URN format correctly with template", func() { + input := "urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod" + template := "/{cluster}/{namespace}/{name}" + result, err := lite.Parse(input, template) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Format).To(Equal(ccrn.FormatURN)) + Expect(result.Fields).To(HaveKeyWithValue("cluster", "eu-de-1")) + }) + + It("returns error for unknown format", func() { + _, err := lite.Parse("unknown:format", "/{name}") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unknown format")) + }) + + It("returns error for empty string", func() { + _, err := lite.Parse("", "") + Expect(err).To(HaveOccurred()) + }) + }) +}) + +var _ = Describe("Round-trip fidelity", func() { + It("CCRN: parse -> format -> re-parse produces same result", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, name=my-pod, namespace=default" + result1, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + + // Format back to CCRN string + formatted := result1.CCRN() + Expect(formatted).ToNot(BeEmpty()) + + // Re-parse + result2, err := lite.ParseCCRN(formatted) + Expect(err).ToNot(HaveOccurred()) + + // Compare + Expect(result2.CCRNKey).To(Equal(result1.CCRNKey)) + Expect(result2.Fields).To(Equal(result1.Fields)) + Expect(result2.Format).To(Equal(result1.Format)) + }) + + It("URN: parse -> format -> re-parse produces same result", func() { + input := "urn:ccrn:pod.k8s.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod" + template := "/{cluster}/{namespace}/{name}" + result1, err := lite.ParseURN(input, template) + Expect(err).ToNot(HaveOccurred()) + + // Format back to URN string + formatted := result1.URN(template) + Expect(formatted).ToNot(BeEmpty()) + + // Re-parse + result2, err := lite.ParseURN(formatted, template) + Expect(err).ToNot(HaveOccurred()) + + // Compare + Expect(result2.CCRNKey).To(Equal(result1.CCRNKey)) + Expect(result2.Fields).To(Equal(result1.Fields)) + Expect(result2.Format).To(Equal(result1.Format)) + }) + + It("cross-format round-trip: CCRN -> URN -> re-parse preserves fields", func() { + input := "ccrn=pod.k8s.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod" + template := "/{cluster}/{namespace}/{name}" + + // Parse as CCRN + parsedCCRN, err := lite.ParseCCRN(input) + Expect(err).ToNot(HaveOccurred()) + + // Format as URN + urnStr := parsedCCRN.URN(template) + Expect(urnStr).ToNot(BeEmpty()) + + // Re-parse as URN + parsedURN, err := lite.ParseURN(urnStr, template) + Expect(err).ToNot(HaveOccurred()) + + // Fields should match + Expect(parsedURN.CCRNKey).To(Equal(parsedCCRN.CCRNKey)) + Expect(parsedURN.Fields).To(Equal(parsedCCRN.Fields)) + }) +}) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 15f59e2..7e62e1a 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -1,23 +1,25 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 package parser import ( "errors" "fmt" - "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" + "io" "strings" + "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" + "github.com/cloudoperators/common-cloud-resource-names/pkg/ccrn" + "github.com/cloudoperators/common-cloud-resource-names/pkg/parser/lite" "github.com/sirupsen/logrus" ) -const DEFAULT_URN_TEMPLATE string = "urn:ccrn:" +const DefaultURNTemplate string = "urn:ccrn:" -// ResourceParser parses both CCRN and URN formats and converts between them, without backend dependencies -// It requires a URN template to parse a URN. +// ResourceParser parses both CCRN and URN formats and converts between them. +// It delegates string parsing to pkg/parser/lite and adds URN template lookup +// from the ValidationBackend. type ResourceParser struct { log *logrus.Logger backend apis.ValidationBackend @@ -25,37 +27,44 @@ type ResourceParser struct { // NewResourceParser creates a new resource parser func NewResourceParser(log *logrus.Logger, backend apis.ValidationBackend) *ResourceParser { + if log == nil { + log = logrus.New() + log.SetOutput(io.Discard) + } return &ResourceParser{log: log, backend: backend} } // Parse parses a CCRN or URN string. For URN, a template must be provided. +// If the template is empty or the default placeholder, the backend is consulted +// to look up the real template. func (p *ResourceParser) Parse(input string, urnTemplate string) (*apis.ParsedResource, error) { if strings.HasPrefix(input, "ccrn=") { - parsed, err := parseCCRNFields(input) + parsed, err := lite.ParseCCRN(input) if err != nil { return nil, err } - return &apis.ParsedResource{ - Format: "CCRN", - Fields: parsed, - Raw: input, - }, nil + return liteToAPIs(parsed), nil } else if strings.HasPrefix(input, "urn:ccrn:") { - if urnTemplate == "" || urnTemplate == DEFAULT_URN_TEMPLATE { - - parsed, err := parseURNCCRNField(input) - + if urnTemplate == "" || urnTemplate == DefaultURNTemplate { + // Need a backend to look up the URN template + if p.backend == nil { + return nil, errors.New("cannot parse URN without a template: no backend configured for template lookup") + } + // Extract the CCRN key to look up the real template from the backend + ccrnKey, err := extractCCRNKeyFromURN(input) if err != nil { return nil, err } - parsedResource := &apis.ParsedResource{ - Format: "URN", - Fields: map[string]string{"ccrn": parsed}, - Raw: input, + // Split ccrnKey into name and version for the backend lookup + parts := strings.SplitN(ccrnKey, "/", 2) + if len(parts) < 2 { + return nil, errors.New("invalid URN format: CCRN key must contain type and version separated by /") } + ccrnName := parts[0] + ccrnVersion := parts[1] - template, err := p.backend.GetURNTemplate(parsedResource.CCRNName(), parsedResource.Version()) + template, err := p.backend.GetURNTemplate(ccrnName, ccrnVersion) if err != nil { return nil, fmt.Errorf("failed to get URN template: %w", err) } @@ -65,52 +74,31 @@ func (p *ResourceParser) Parse(input string, urnTemplate string) (*apis.ParsedRe if !strings.HasPrefix(urnTemplate, "urn:ccrn:") { return nil, errors.New("invalid URN template: must start with 'urn:ccrn:'") } - parsed, err := parseURNFields(input, urnTemplate) + + // Convert the backend template format (urn:ccrn://) + // to the lite parser template format (/{field1}/{field2}) + liteTemplate := convertToLiteTemplate(urnTemplate) + + parsed, err := lite.ParseURN(input, liteTemplate) if err != nil { return nil, err } - return &apis.ParsedResource{ - Format: "URN", - Fields: parsed, - Raw: input, - }, nil + return liteToAPIs(parsed), nil } return nil, errors.New("unknown format: must start with 'ccrn=' or 'urn:ccrn:'") } -// parseCCRNFields parses a CCRN string into fields -func parseCCRNFields(ccrn string) (map[string]string, error) { - if !strings.HasPrefix(ccrn, "ccrn=") { - return nil, errors.New("invalid CCRN format: must start with 'ccrn='") - } - fieldsPart := strings.TrimSpace(ccrn) - fields := make(map[string]string) - fieldEntries := strings.Split(fieldsPart, ",") - for _, entry := range fieldEntries { - entry = strings.TrimSpace(entry) - if entry == "" { - continue - } - parts := strings.SplitN(entry, "=", 2) - if len(parts) != 2 { - return nil, errors.New("invalid field format: " + entry + " (must be key=value)") - } - key := strings.TrimSpace(parts[0]) - value := strings.TrimSpace(parts[1]) - if len(value) >= 2 && value[0] == '"' && value[len(value)-1] == '"' { - value = value[1 : len(value)-1] - } - fields[key] = value - } - if _, exists := fields["ccrn"]; !exists { - return nil, errors.New("missing required field: ccrn") - } - return fields, nil +// ExtractCCRNKeyFromURN extracts the CCRN key from a URN string. +func (p *ResourceParser) ExtractCCRNKeyFromURN(urn string) (string, error) { + return extractCCRNKeyFromURN(urn) } -func parseURNCCRNField(urn string) (string, error) { - // Remove prefix +// extractCCRNKeyFromURN extracts the CCRN key (type/version) from a URN string. +func extractCCRNKeyFromURN(urn string) (string, error) { body := strings.TrimPrefix(urn, "urn:ccrn:") + if body == "" { + return "", errors.New("invalid URN format: empty body after 'urn:ccrn:' prefix") + } parts := strings.Split(body, "/") if len(parts) < 3 { return "", errors.New("invalid URN format: must contain at least three segments after 'urn:ccrn:'") @@ -118,55 +106,56 @@ func parseURNCCRNField(urn string) (string, error) { return parts[0] + "/" + parts[1], nil } -// parseURNFields parses a URN string into fields using the provided template -func parseURNFields(urn, urnTemplate string) (map[string]string, error) { - if !strings.HasPrefix(urn, "urn:ccrn:") { - return nil, errors.New("invalid URN format: must start with 'urn:ccrn:'") - } - if !strings.HasPrefix(urnTemplate, "urn:ccrn:") { - return nil, errors.New("invalid URN template: must start with 'urn:ccrn:'") - } - // Remove prefix - body := strings.TrimPrefix(urn, "urn:ccrn:") - templateBody := strings.TrimPrefix(urnTemplate, "urn:ccrn:") - templateParts := strings.Split(templateBody, "/") - - // The first element is the ccrn type/version so we rebuild the parts accordingly, the last part can be an path with slashes - tmpParts := strings.SplitN(body, "/", len(templateParts)+1) - parts := make([]string, len(tmpParts)-1) - parts[0] = tmpParts[0] + "/" + tmpParts[1] - for i := 2; i < len(tmpParts); i++ { - if tmpParts[i] != "" { - parts[i-1] = tmpParts[i] - } - } - - if len(parts) < len(templateParts) { - return nil, errors.New("URN and template do not match in segment count. Expected format " + urnTemplate + " segments, got: " + urn) +// convertToLiteTemplate converts a backend URN template +// (e.g., "urn:ccrn:///") +// to the lite parser template format (e.g., "/{cluster}/{namespace}/{name}"). +// The placeholder occupies one segment in the template string, even though +// it expands to "type.group/version" (two slash-separated parts) at runtime. +func convertToLiteTemplate(backendTemplate string) string { + // Strip the urn:ccrn: prefix + body := strings.TrimPrefix(backendTemplate, "urn:ccrn:") + + // Split by / + segments := strings.Split(body, "/") + + // Skip the first segment which is the placeholder + var fieldSegments []string + if len(segments) > 1 { + fieldSegments = segments[1:] + } else { + return "" } - fields := make(map[string]string) - for i, t := range templateParts { - if strings.HasPrefix(t, "<") && strings.HasSuffix(t, ">") { - key := t[1 : len(t)-1] - fields[key] = parts[i] - } else if t == "" { - fields["ccrn"] = parts[i] - } else if t != parts[i] { - return nil, fmt.Errorf("URN segment '%s' does not match template '%s'", parts[i], t) + // Convert to {field} + var result strings.Builder + for _, seg := range fieldSegments { + if seg == "" { + continue + } + result.WriteString("/") + if strings.HasPrefix(seg, "<") && strings.HasSuffix(seg, ">") { + fieldName := seg[1 : len(seg)-1] + result.WriteString("{") + result.WriteString(fieldName) + result.WriteString("}") + } else { + result.WriteString(seg) } } - if _, exists := fields["ccrn"]; !exists { - return nil, errors.New("missing required field: ccrn") - } - return fields, nil + return result.String() } -// ExtractCCRNKeyFromURN extracts the CCRN key from a URN using the template -func (p *ResourceParser) ExtractCCRNKeyFromURN(urn string) (string, error) { - ccrn, err := parseURNCCRNField(urn) - if err != nil { - return "", err +// liteToAPIs converts a ccrn.ParsedResource from the lite parser into an apis.ParsedResource. +// The CCRNKey is stored in the fields map under the "ccrn" key for backward compatibility. +func liteToAPIs(parsed *ccrn.ParsedResource) *apis.ParsedResource { + fields := make(map[string]string, len(parsed.Fields)+1) + fields["ccrn"] = parsed.CCRNKey + for k, v := range parsed.Fields { + fields[k] = v + } + return &apis.ParsedResource{ + Format: parsed.Format, + Fields: fields, + Raw: parsed.RawInput, } - return ccrn, nil } diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go new file mode 100644 index 0000000..6017b4d --- /dev/null +++ b/pkg/parser/parser_test.go @@ -0,0 +1,84 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package parser_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/parser" +) + +func TestParser(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Parser Suite") +} + +var _ = Describe("ResourceParser", func() { + Context("nil logger handling", func() { + It("should not panic when created with nil logger", func() { + // A nil logger should not cause a panic when the parser is used + Expect(func() { + p := parser.NewResourceParser(nil, nil) + Expect(p).ToNot(BeNil()) + }).ToNot(Panic()) + }) + }) + + Context("URN parsing bounds checking", func() { + It("should not panic on empty URN after prefix", func() { + p := parser.NewResourceParser(nil, nil) + Expect(func() { + _, err := p.Parse("urn:ccrn:", parser.DefaultURNTemplate) + Expect(err).To(HaveOccurred()) + }).ToNot(Panic()) + }) + + It("should not panic on URN with only one segment", func() { + p := parser.NewResourceParser(nil, nil) + Expect(func() { + _, err := p.Parse("urn:ccrn:foo", parser.DefaultURNTemplate) + Expect(err).To(HaveOccurred()) + }).ToNot(Panic()) + }) + + It("should not panic on URN with two segments", func() { + p := parser.NewResourceParser(nil, nil) + Expect(func() { + _, err := p.Parse("urn:ccrn:foo/bar", parser.DefaultURNTemplate) + Expect(err).To(HaveOccurred()) + }).ToNot(Panic()) + }) + + It("should not panic on malformed URN with template", func() { + p := parser.NewResourceParser(nil, nil) + Expect(func() { + _, err := p.Parse("urn:ccrn:x", "urn:ccrn://") + Expect(err).To(HaveOccurred()) + }).ToNot(Panic()) + }) + + It("should not panic on URN with insufficient segments for template", func() { + p := parser.NewResourceParser(nil, nil) + Expect(func() { + _, err := p.Parse("urn:ccrn:a/b", "urn:ccrn://") + Expect(err).To(HaveOccurred()) + }).ToNot(Panic()) + }) + + It("should return error for ExtractCCRNKeyFromURN with malformed input", func() { + p := parser.NewResourceParser(nil, nil) + _, err := p.ExtractCCRNKeyFromURN("urn:ccrn:") + Expect(err).To(HaveOccurred()) + }) + + It("should return error for ExtractCCRNKeyFromURN with only one segment", func() { + p := parser.NewResourceParser(nil, nil) + _, err := p.ExtractCCRNKeyFromURN("urn:ccrn:foo") + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/pkg/validation/filesystem_backend.go b/pkg/validation/filesystem_backend.go index 01050ea..3903bcc 100644 --- a/pkg/validation/filesystem_backend.go +++ b/pkg/validation/filesystem_backend.go @@ -1,8 +1,6 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 package validation import ( @@ -148,33 +146,48 @@ func (fb *FilesystemBackend) LoadCRDs(pattern string) error { func (fb *FilesystemBackend) LoadCRDsFromDirectory(dir string) error { fb.log.Infof("Loading CRDs from directory: %s", dir) - // Search patterns for both recursive and non-recursive - patterns := []string{ - filepath.Join(dir, "*.yaml"), - filepath.Join(dir, "*.yml"), - filepath.Join(dir, "**", "*.yaml"), - filepath.Join(dir, "**", "*.yml"), + // Walk the directory tree to find all YAML files + var yamlFiles []string + err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && fb.isYAMLFile(path) { + yamlFiles = append(yamlFiles, path) + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to walk directory %s: %w", dir, err) } - var allErrors []error - totalLoaded := 0 + if len(yamlFiles) == 0 { + return fmt.Errorf("failed to load CRDs from directory %s: no YAML files found", dir) + } - // Try each pattern and accumulate results - for _, pattern := range patterns { - if err := fb.LoadCRDs(pattern); err != nil { - allErrors = append(allErrors, fmt.Errorf("pattern %s: %w", pattern, err)) - } else { - // Count how many CRDs we have now to track progress - fb.crdsMutex.RLock() - currentCount := len(fb.crds) - fb.crdsMutex.RUnlock() - totalLoaded = currentCount - } + // Process all found YAML files + result := &CRDLoadingResult{ + Errors: make([]error, 0), + LoadedCRDKeys: make([]string, 0), } - // If no CRDs were loaded from any pattern, return combined errors - if totalLoaded == 0 && len(allErrors) > 0 { - return fmt.Errorf("failed to load CRDs from directory %s: %w", dir, errors.Join(allErrors...)) + for _, filePath := range yamlFiles { + fb.processFile(filePath, result) + } + + // Store the directory for potential refresh operations + fb.loadedPaths = append(fb.loadedPaths, dir) + + // Log comprehensive results + fb.logLoadingResults(result) + + // Return error only if no CRDs were loaded at all + if result.ProcessedCRDs == 0 && len(result.Errors) > 0 { + return fmt.Errorf("failed to load CRDs from directory %s: %w", dir, errors.Join(result.Errors...)) + } + + if len(result.Errors) > 0 { + fb.log.Warnf("Some errors occurred during CRD loading from directory, but %d CRDs loaded successfully", result.ProcessedCRDs) } return nil @@ -650,10 +663,18 @@ func (fb *FilesystemBackend) Refresh() error { fb.crdsMutex.Unlock() // Reload from all previously loaded paths + // Paths may be glob patterns (from LoadCRDs) or directories (from LoadCRDsFromDirectory) var allErrors []error for _, path := range fb.loadedPaths { - if err := fb.LoadCRDs(path); err != nil { - allErrors = append(allErrors, fmt.Errorf("failed to refresh path %s: %w", path, err)) + info, statErr := os.Stat(path) + if statErr == nil && info.IsDir() { + if err := fb.LoadCRDsFromDirectory(path); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to refresh directory %s: %w", path, err)) + } + } else { + if err := fb.LoadCRDs(path); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to refresh path %s: %w", path, err)) + } } } diff --git a/pkg/validation/filesystem_backend_test.go b/pkg/validation/filesystem_backend_test.go index fb9b9a7..17a41b2 100644 --- a/pkg/validation/filesystem_backend_test.go +++ b/pkg/validation/filesystem_backend_test.go @@ -1,8 +1,6 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 package validation_test import ( @@ -54,16 +52,35 @@ var _ = Describe("FilesystemBackend", func() { Expect(backend.GetLoadedCRDs()).To(ContainElement("testresource.tr.ccrn.example.com/v1")) }) + It("loads CRDs from nested subdirectories", func() { + // Arrange - testdata/subdir/nested_crd.yaml should be found recursively + // Act + err := backend.LoadCRDsFromDirectory(filepath.Join("testdata")) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(backend.GetLoadedCRDs()).To(ContainElement("nestedresource.tr.ccrn.example.com/v1")) + Expect(backend.GetLoadedCRDs()).To(ContainElement("testresource.tr.ccrn.example.com/v1")) + }) + It("returns error for non-existent directory", func() { // Act err := backend.LoadCRDsFromDirectory("/non/existent/path") // Assert Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failed to load CRDs from directory")) }) }) Context("LoadCRDs", func() { + It("does not panic when created with nil logger", func() { + // Test that NewOfflineBackend handles nil logger gracefully + Expect(func() { + b := validation.NewOfflineBackend(nil, "ccrn.example.com") + Expect(b).ToNot(BeNil()) + // Verify it can actually be used without panics + _ = b.IsResourceTypeSupported("foo") + }).ToNot(Panic()) + }) + It("returns error for invalid YAML", func() { // Arrange invalidPath := filepath.Join(tempDir, "invalid.yaml") @@ -108,7 +125,7 @@ var _ = Describe("FilesystemBackend", func() { Expect(err.Error()).To(ContainSubstring("failed to parse YAML")) }) - It("returns error if both directory patterns fail in LoadCRDsFromDirectory", func() { + It("returns error if directory is empty (no YAML files) in LoadCRDsFromDirectory", func() { // Arrange dir := filepath.Join(os.TempDir(), "emptydir") os.MkdirAll(dir, 0755) @@ -117,7 +134,7 @@ var _ = Describe("FilesystemBackend", func() { err := backend.LoadCRDsFromDirectory(dir) // Assert Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no files found matching pattern")) + Expect(err.Error()).To(ContainSubstring("no YAML files found")) }) }) @@ -133,6 +150,16 @@ var _ = Describe("FilesystemBackend", func() { validator = validation.NewCCRNValidator(backend) }) + It("produces properly formatted error messages for unknown URN CRD", func() { + // This tests the fix for the broken []string{"%s", val1, val2} pattern + result, _ := validator.ValidateCCRN("urn:ccrn:unknown.tr.ccrn.example.com/v1/foo") + Expect(result.Valid).To(BeFalse()) + if len(result.Errors) > 0 { + // Error should be a properly formatted string, not contain literal %s + Expect(result.Errors[0]).ToNot(ContainSubstring("%s")) + } + }) + DescribeTable("validates CCRNs correctly", func(ccrn string, expected bool) { // Act diff --git a/pkg/validation/interface.go b/pkg/validation/interface.go index 1915f60..992f706 100644 --- a/pkg/validation/interface.go +++ b/pkg/validation/interface.go @@ -1,7 +1,4 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package validation diff --git a/pkg/validation/kubernetes_backend.go b/pkg/validation/kubernetes_backend.go index 18231fa..3a46c66 100644 --- a/pkg/validation/kubernetes_backend.go +++ b/pkg/validation/kubernetes_backend.go @@ -1,14 +1,12 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package validation import ( "context" "fmt" + "io" "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" @@ -42,6 +40,11 @@ type KubernetesBackend struct { // NewKubernetesBackend creates a new Kubernetes validation backend func NewKubernetesBackend(config *rest.Config, log *logrus.Logger, ccrnGroup string) (*KubernetesBackend, error) { + if log == nil { + log = logrus.New() + log.SetOutput(io.Discard) + } + // Create Kubernetes client kubeClient, err := kubernetes.NewForConfig(config) if err != nil { @@ -102,11 +105,12 @@ func (kb *KubernetesBackend) GetCRD(crdVersion string) (*apis.CRDInfo, error) { return crdInfo, nil } -// ValidateResource validates a resource by creating it in the Kubernetes cluster +// ValidateResource validates a resource using dry-run create against the Kubernetes API server. +// This validates via the API server without persisting the resource. func (kb *KubernetesBackend) ValidateResource(namespace string, parsedCCRN *apis.ParsedResource) error { // Get CRD info - group := parsedCCRN.ApiGroup() + group := parsedCCRN.APIGroup() version := parsedCCRN.Version() kind := parsedCCRN.GetKind() @@ -128,12 +132,14 @@ func (kb *KubernetesBackend) ValidateResource(namespace string, parsedCCRN *apis Resource: crdInfo.Plural, } - // Create the resource - kb.log.WithField("resource", resourceObj).Infof("Creating resource %s/%s", namespace, resourceName) + // Validate the resource using dry-run create (no resource is persisted) + kb.log.WithField("resource", resourceObj).Infof("Dry-run validating resource %s/%s", namespace, resourceName) resourceClient := kb.dynamicClient.Resource(gvr).Namespace(namespace) - _, err = resourceClient.Create(context.TODO(), &unstructured.Unstructured{Object: resourceObj}, metav1.CreateOptions{}) + _, err = resourceClient.Create(context.TODO(), &unstructured.Unstructured{Object: resourceObj}, metav1.CreateOptions{ + DryRun: []string{metav1.DryRunAll}, + }) if err != nil { - return fmt.Errorf("failed to create resource: %w", err) + return fmt.Errorf("failed to validate resource (dry-run): %w", err) } return nil diff --git a/pkg/validation/testdata/subdir/nested_crd.yaml b/pkg/validation/testdata/subdir/nested_crd.yaml new file mode 100644 index 0000000..bc19416 --- /dev/null +++ b/pkg/validation/testdata/subdir/nested_crd.yaml @@ -0,0 +1,35 @@ +# SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +# SPDX-License-Identifier: Apache-2.0 + +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: nestedresource.tr.ccrn.example.com + annotations: + ccrn/v1.urn-template: "urn:ccrn:/" +spec: + group: tr.ccrn.example.com + names: + kind: NestedResource + listKind: NestedResourceList + plural: nestedresources + singular: nestedresource + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + required: ["ccrn", "name"] + properties: + ccrn: + type: string + enum: ["nestedresource.tr.ccrn.example.com/v1"] + description: "API version of the resource" + name: + type: string + description: "The name of the resource" + pattern: "^([a-z0-9]([-a-z0-9]*[a-z0-9])?|\\*)$" + maxLength: 253 diff --git a/pkg/validation/validator.go b/pkg/validation/validator.go index ff79e9c..ae25a5a 100644 --- a/pkg/validation/validator.go +++ b/pkg/validation/validator.go @@ -1,13 +1,11 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package validation -import "C" import ( + "fmt" + "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" "github.com/cloudoperators/common-cloud-resource-names/pkg/parser" ) @@ -28,7 +26,7 @@ func NewCCRNValidator(backend apis.ValidationBackend) *CCRNValidator { // ValidateCCRN validates a CCRN string func (v *CCRNValidator) ValidateCCRN(ccrnStr string) (*apis.ValidationResult, error) { - parsed, err := v.parser.Parse(ccrnStr, parser.DEFAULT_URN_TEMPLATE) + parsed, err := v.parser.Parse(ccrnStr, parser.DefaultURNTemplate) if err != nil { return &apis.ValidationResult{ Valid: false, @@ -42,7 +40,7 @@ func (v *CCRNValidator) ValidateCCRN(ccrnStr string) (*apis.ValidationResult, er return &apis.ValidationResult{ Valid: false, ParsedCCRN: parsed, - Errors: []string{"A CCRN definition for %s could not be retrieved: %s", parsed.CCRNKey(), err.Error()}, + Errors: []string{fmt.Sprintf("A CCRN definition for %s could not be retrieved: %s", parsed.CCRNKey(), err.Error())}, }, err } parsed, err = v.parser.Parse(ccrnStr, info.URNFormat) diff --git a/pkg/webhook/admission_test.go b/pkg/webhook/admission_test.go new file mode 100644 index 0000000..dccf39a --- /dev/null +++ b/pkg/webhook/admission_test.go @@ -0,0 +1,464 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package webhook + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" + + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" +) + +func TestAdmissionHandler(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Admission Handler Suite") +} + +// admissionMockBackend implements apis.ValidationBackend with configurable responses +type admissionMockBackend struct { + // GetCRD configuration + getCRDResponse *apis.CRDInfo + getCRDErr error + + // ValidateResource configuration + validateResourceErr error + + // GetURNTemplate configuration + getURNTemplateResponse string + getURNTemplateErr error + + // Refresh configuration + refreshErr error + + // IsResourceTypeSupported configuration + isResourceTypeSupported bool +} + +func (m *admissionMockBackend) GetCRD(ccrnVersion string) (*apis.CRDInfo, error) { + return m.getCRDResponse, m.getCRDErr +} + +func (m *admissionMockBackend) ValidateResource(namespace string, parsedCCRN *apis.ParsedResource) error { + return m.validateResourceErr +} + +func (m *admissionMockBackend) GetURNTemplate(ccrnName string, ccrnVersion string) (string, error) { + return m.getURNTemplateResponse, m.getURNTemplateErr +} + +func (m *admissionMockBackend) Refresh() error { + return m.refreshErr +} + +func (m *admissionMockBackend) IsResourceTypeSupported(ccrnVersion string) bool { + return m.isResourceTypeSupported +} + +// buildAdmissionReviewBody creates an AdmissionReview JSON request body from a CCRN spec. +// The ccrn value should be the full CCRN string (e.g., "ccrn=type.group/version, field=value"). +func buildAdmissionReviewBody(ccrn, urn string) ([]byte, error) { + ccrnObj := apis.CCRN{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "ccrn.cloudoperators.dev/v1", + Kind: "CCRN", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-resource", + Namespace: "default", + }, + Spec: apis.CCRNSpec{ + CCRN: ccrn, + URN: urn, + }, + } + + rawObj, err := json.Marshal(ccrnObj) + if err != nil { + return nil, err + } + + review := admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "admission.k8s.io/v1", + Kind: "AdmissionReview", + }, + Request: &admissionv1.AdmissionRequest{ + UID: types.UID("test-uid-12345"), + Name: "test-resource", + Namespace: "default", + Object: runtime.RawExtension{ + Raw: rawObj, + }, + }, + } + + return json.Marshal(review) +} + +// performAdmissionRequest sends a POST to /validate and parses the AdmissionReview response. +func performAdmissionRequest(server *WebhookServer, body []byte) (*admissionv1.AdmissionReview, int) { + req := httptest.NewRequest(http.MethodPost, "/validate", strings.NewReader(string(body))) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + server.mutateCCRN(w, req) + + result := w.Result() + statusCode := result.StatusCode + + if statusCode != http.StatusOK { + return nil, statusCode + } + + var review admissionv1.AdmissionReview + err := json.NewDecoder(result.Body).Decode(&review) + if err != nil { + return nil, statusCode + } + + return &review, statusCode +} + +var _ = Describe("Admission Handler", func() { + var ( + server *WebhookServer + backend *admissionMockBackend + log *logrus.Logger + ) + + BeforeEach(func() { + log = logrus.New() + log.SetOutput(GinkgoWriter) + + backend = &admissionMockBackend{ + isResourceTypeSupported: true, + getURNTemplateResponse: "urn:ccrn:///", + } + }) + + JustBeforeEach(func() { + var err error + server, err = NewWebhookServer(log, backend) + Expect(err).NotTo(HaveOccurred()) + }) + + Describe("Valid CCRN submission", func() { + It("should allow the request and generate a URN mutation patch", func() { + // spec.ccrn stores the full CCRN string including the "ccrn=" prefix + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeTrue()) + Expect(review.Response.UID).To(Equal(types.UID("test-uid-12345"))) + + // Verify mutation patch is present (URN should be added) + Expect(review.Response.Patch).NotTo(BeNil()) + Expect(review.Response.PatchType).NotTo(BeNil()) + Expect(*review.Response.PatchType).To(Equal(admissionv1.PatchTypeJSONPatch)) + + // Parse and verify patch content + var patches []map[string]interface{} + err = json.Unmarshal(review.Response.Patch, &patches) + Expect(err).NotTo(HaveOccurred()) + Expect(patches).To(HaveLen(1)) + Expect(patches[0]["op"]).To(Equal("add")) + Expect(patches[0]["path"]).To(Equal("/spec/urn")) + // The value should be a URN string + urnValue, ok := patches[0]["value"].(string) + Expect(ok).To(BeTrue()) + Expect(urnValue).To(HavePrefix("urn:ccrn:")) + }) + }) + + Describe("Valid URN submission", func() { + It("should parse the URN and attempt CCRN validation from derived key", func() { + body, err := buildAdmissionReviewBody( + "", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.UID).To(Equal(types.UID("test-uid-12345"))) + + // The URN path extracts the CCRN key and passes it to ValidateCCRN. + // The extracted key ("pod.k8s.../v1") does not have the "ccrn=" prefix, + // so the validator's parser rejects it. This is the current behavior. + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result.Message).To(ContainSubstring("Derived CCRN validation error")) + }) + + Context("when the extracted key is prefixed with ccrn=", func() { + It("should parse URN template segments correctly", func() { + // Test that the URN path correctly calls GetURNTemplate with the right arguments + // and parses the URN using that template. Even though the final validation step + // fails, the parsing portion works. + body, err := buildAdmissionReviewBody( + "", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + // Confirms the URN was parsed successfully (no "Failed to parse URN" error) + // and the CCRN extraction succeeded (no "Failed to extract CCRN from URN" error) + // The error is specifically at the ValidateCCRN step + Expect(review.Response.Result.Message).NotTo(ContainSubstring("Failed to parse URN")) + Expect(review.Response.Result.Message).NotTo(ContainSubstring("Failed to extract CCRN")) + Expect(review.Response.Result.Message).To(ContainSubstring("Derived CCRN validation error")) + }) + }) + }) + + Describe("Both CCRN and URN present", func() { + It("should allow the request with no mutation patch needed", func() { + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeTrue()) + + // No mutation needed since both are present + Expect(review.Response.Patch).To(BeNil()) + Expect(review.Response.Result.Message).To(Equal("CCRN is valid and target resource validated")) + }) + }) + + Describe("Neither CCRN nor URN present", func() { + It("should reject the request with a clear error", func() { + body, err := buildAdmissionReviewBody("", "") + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result).NotTo(BeNil()) + Expect(review.Response.Result.Message).To(ContainSubstring("must have either spec.ccrn or spec.urn defined")) + }) + }) + + Describe("Invalid CCRN format", func() { + It("should reject with a validation error for unrecognized format", func() { + // "invalidformat" does not start with "ccrn=" or "urn:ccrn:" so the parser rejects it + body, err := buildAdmissionReviewBody("invalidformat", "") + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result).NotTo(BeNil()) + Expect(review.Response.Result.Status).To(Equal("Failure")) + Expect(review.Response.Result.Message).To(ContainSubstring("CCRN validation error")) + }) + + It("should reject a CCRN with empty value after prefix", func() { + // "ccrn=" with no value - the parser will likely fail + body, err := buildAdmissionReviewBody("ccrn=", "") + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result).NotTo(BeNil()) + Expect(review.Response.Result.Status).To(Equal("Failure")) + }) + }) + + Describe("Unsupported resource type", func() { + BeforeEach(func() { + backend = &admissionMockBackend{ + isResourceTypeSupported: false, + getURNTemplateResponse: "urn:ccrn:///", + } + }) + + It("should reject with unsupported resource type error", func() { + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result).NotTo(BeNil()) + Expect(review.Response.Result.Message).To(ContainSubstring("Resource type not supported")) + }) + }) + + Describe("Backend validation failure", func() { + BeforeEach(func() { + backend = &admissionMockBackend{ + isResourceTypeSupported: true, + validateResourceErr: fmt.Errorf("backend connection timeout"), + getURNTemplateResponse: "urn:ccrn:///", + } + }) + + It("should reject with backend error message", func() { + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result).NotTo(BeNil()) + Expect(review.Response.Result.Message).To(ContainSubstring("backend connection timeout")) + }) + }) + + Describe("Malformed request body", func() { + It("should return HTTP 400 for invalid JSON", func() { + req := httptest.NewRequest(http.MethodPost, "/validate", strings.NewReader("not json")) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + server.mutateCCRN(w, req) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("should reject unparseable CCRN object raw", func() { + // Construct the AdmissionReview JSON manually with invalid object.raw + // that will fail json.Unmarshal into a CCRN struct. + // Using invalid JSON bytes that k8s RawExtension will store but CCRN unmarshal will reject. + rawBody := `{ + "apiVersion": "admission.k8s.io/v1", + "kind": "AdmissionReview", + "request": { + "uid": "test-uid-malformed", + "object": {"not_a_valid_field": true} + } + }` + + req := httptest.NewRequest(http.MethodPost, "/validate", strings.NewReader(rawBody)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + server.mutateCCRN(w, req) + + // When the AdmissionReview is parsed, request.Object.Raw will be nil + // because the "object" field is not in the expected RawExtension format. + // This triggers "neither CCRN nor URN" error since the CCRN struct is zero-valued. + Expect(w.Code).To(Equal(http.StatusOK)) + var resp admissionv1.AdmissionReview + err := json.NewDecoder(w.Result().Body).Decode(&resp) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.Response.Allowed).To(BeFalse()) + }) + }) + + Describe("URN template lookup failure", func() { + BeforeEach(func() { + backend = &admissionMockBackend{ + isResourceTypeSupported: true, + getURNTemplateErr: fmt.Errorf("CRD not found"), + } + }) + + It("should reject when URN template cannot be retrieved", func() { + body, err := buildAdmissionReviewBody( + "", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review).NotTo(BeNil()) + Expect(review.Response).NotTo(BeNil()) + Expect(review.Response.Allowed).To(BeFalse()) + Expect(review.Response.Result.Message).To(ContainSubstring("CRD not found")) + }) + }) + + Describe("Response UID propagation", func() { + It("should propagate the request UID to the response", func() { + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + review, statusCode := performAdmissionRequest(server, body) + + Expect(statusCode).To(Equal(http.StatusOK)) + Expect(review.Response.UID).To(Equal(types.UID("test-uid-12345"))) + }) + }) + + Describe("Content-Type header", func() { + It("should set Content-Type to application/json in the response", func() { + body, err := buildAdmissionReviewBody( + "ccrn=pod.k8s-registry.ccrn.cloudoperators.dev/v1, cluster=eu-de-1, namespace=default, name=my-pod", + "urn:ccrn:pod.k8s-registry.ccrn.cloudoperators.dev/v1/eu-de-1/default/my-pod", + ) + Expect(err).NotTo(HaveOccurred()) + + req := httptest.NewRequest(http.MethodPost, "/validate", strings.NewReader(string(body))) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + + server.mutateCCRN(w, req) + + Expect(w.Header().Get("Content-Type")).To(Equal("application/json")) + }) + }) +}) diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 7cd469f..5c7b611 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -1,17 +1,17 @@ // SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors // SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: SAP SE or an SAP affiliate company -// SPDX-License-Identifier: Apache-2.0 - package webhook import ( + "context" "encoding/json" "fmt" "io" + "net" "net/http" "strings" + "sync/atomic" "time" "github.com/sirupsen/logrus" @@ -27,14 +27,21 @@ import ( // WebhookServer implements the admission webhook for CCRN validation type WebhookServer struct { - log *logrus.Logger - validator *validation.CCRNValidator - backend apis.ValidationBackend - parser *parser.ResourceParser + log *logrus.Logger + validator *validation.CCRNValidator + backend apis.ValidationBackend + parser *parser.ResourceParser + httpServer *http.Server + ready atomic.Bool } // NewWebhookServer creates a new webhook server using the provided validation backend func NewWebhookServer(log *logrus.Logger, backend apis.ValidationBackend) (*WebhookServer, error) { + if log == nil { + log = logrus.New() + log.SetOutput(io.Discard) + } + server := &WebhookServer{ log: log, validator: validation.NewCCRNValidator(backend), @@ -42,6 +49,14 @@ func NewWebhookServer(log *logrus.Logger, backend apis.ValidationBackend) (*Webh parser: parser.NewResourceParser(log, backend), } + // Initialize the HTTP mux and server + mux := http.NewServeMux() + mux.HandleFunc("/validate", server.mutateCCRN) + mux.HandleFunc("/healthz", server.healthz) + server.httpServer = &http.Server{ + Handler: mux, + } + return server, nil } @@ -65,20 +80,36 @@ func NewWebhookServerFromConfig(log *logrus.Logger, ccrnGroup string) (*WebhookS return NewWebhookServer(log, backend) } -// Serve starts the webhook server +// Serve starts the webhook server with TLS func (s *WebhookServer) Serve(port int, certFile, keyFile string) error { - // Setup the HTTP server - mux := http.NewServeMux() - mux.HandleFunc("/validate", s.mutateCCRN) - mux.HandleFunc("/healthz", s.healthz) + s.httpServer.Addr = fmt.Sprintf(":%d", port) + s.log.Infof("Starting webhook server on port %d", port) + return s.httpServer.ListenAndServeTLS(certFile, keyFile) +} - server := &http.Server{ - Addr: fmt.Sprintf(":%d", port), - Handler: mux, +// ServeHTTP starts the webhook server without TLS (for testing). +// It accepts a net.Listener so callers can bind to port 0 for ephemeral ports. +func (s *WebhookServer) ServeHTTP(ln net.Listener) error { + s.log.Infof("Starting webhook server (HTTP) on %s", ln.Addr()) + return s.httpServer.Serve(ln) +} + +// Shutdown gracefully shuts down the webhook server, allowing in-flight requests to complete. +func (s *WebhookServer) Shutdown(ctx context.Context) error { + if s.httpServer == nil { + return nil } + return s.httpServer.Shutdown(ctx) +} - s.log.Infof("Starting webhook server on port %d", port) - return server.ListenAndServeTLS(certFile, keyFile) +// SetReady marks the server as ready to serve traffic +func (s *WebhookServer) SetReady() { + s.ready.Store(true) +} + +// IsReady returns whether the server is ready to serve traffic +func (s *WebhookServer) IsReady() bool { + return s.ready.Load() } // mutateCCRN is the HTTP handler for webhook mutation requests @@ -160,7 +191,7 @@ func (s *WebhookServer) handleCombinedRequest(request *admissionv1.AdmissionRequ Allowed: true, Result: &metav1.Status{ Status: "Success", - Message: "CCRN is valid and target resource created", + Message: "CCRN is valid and target resource validated", }, } @@ -172,7 +203,7 @@ func (s *WebhookServer) handleCombinedRequest(request *admissionv1.AdmissionRequ pt := admissionv1.PatchTypeJSONPatch response.Patch = patchBytes response.PatchType = &pt - response.Result.Message = "CCRN is valid, missing format added, and target resource created" + response.Result.Message = "CCRN is valid, missing format added, and target resource validated" } } @@ -302,7 +333,7 @@ func (s *WebhookServer) generateMutationPatches(ccrn *apis.CCRN, parsedCCRN *api s.log.Infof("CCRN is present, generating URN from CCRN") template, err := s.backend.GetURNTemplate(parsedCCRN.CCRNName(), parsedCCRN.Version()) if err != nil { - s.log.Errorf("Failed to get URN template for %s/%s: %v", parsedCCRN.ApiGroup(), parsedCCRN.Version(), err) + s.log.Errorf("Failed to get URN template for %s/%s: %v", parsedCCRN.APIGroup(), parsedCCRN.Version(), err) return nil, false } urn := parsedCCRN.URN(template) @@ -321,7 +352,7 @@ func (s *WebhookServer) generateMutationPatches(ccrn *apis.CCRN, parsedCCRN *api } else if ccrn.Spec.URN != "" && ccrn.Spec.CCRN == "" { s.log.Infof("URN is present, generating CCRN from URN") // Validate URN and derive CCRN - parsedURN, err := s.parser.Parse(ccrn.Spec.URN, parser.DEFAULT_URN_TEMPLATE) // Use default template to get the ccrn field + parsedURN, err := s.parser.Parse(ccrn.Spec.URN, parser.DefaultURNTemplate) // Use default template to get the ccrn field if err != nil { s.log.Errorf("Failed to parse URN using default template: %v", err) return nil, false @@ -339,8 +370,16 @@ func (s *WebhookServer) generateMutationPatches(ccrn *apis.CCRN, parsedCCRN *api return patches, len(patches) > 0 } -// healthz is the health check endpoint +// healthz is the health check endpoint. Returns 503 if the server is not ready, +// 200 otherwise. func (s *WebhookServer) healthz(w http.ResponseWriter, r *http.Request) { + if !s.ready.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + if _, err := w.Write([]byte("not ready")); err != nil { + s.log.Errorf("Failed to write response: %v", err) + } + return + } w.WriteHeader(http.StatusOK) if _, err := w.Write([]byte("ok")); err != nil { s.log.Errorf("Failed to write response: %v", err) diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go new file mode 100644 index 0000000..2ef2811 --- /dev/null +++ b/pkg/webhook/webhook_test.go @@ -0,0 +1,247 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +// SPDX-License-Identifier: Apache-2.0 + +package webhook + +import ( + "context" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/sirupsen/logrus" + + "github.com/cloudoperators/common-cloud-resource-names/pkg/apis" +) + +// mockBackend implements apis.ValidationBackend for testing +type mockBackend struct { + refreshErr error +} + +func (m *mockBackend) GetCRD(ccrnVersion string) (*apis.CRDInfo, error) { + return nil, nil +} + +func (m *mockBackend) ValidateResource(namespace string, parsedCCRN *apis.ParsedResource) error { + return nil +} + +func (m *mockBackend) GetURNTemplate(ccrnName string, ccrnVersion string) (string, error) { + return "", nil +} + +func (m *mockBackend) Refresh() error { + return m.refreshErr +} + +func (m *mockBackend) IsResourceTypeSupported(ccrnVersion string) bool { + return false +} + +func TestHealthzReturns503BeforeReady(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/healthz", nil) + w := httptest.NewRecorder() + + server.healthz(w, req) + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("expected status 503 before ready, got %d", w.Code) + } +} + +func TestHealthzReturns200AfterReady(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + + // Mark server as ready + server.SetReady() + + req := httptest.NewRequest(http.MethodGet, "/healthz", nil) + w := httptest.NewRecorder() + + server.healthz(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200 after ready, got %d", w.Code) + } +} + +func TestShutdownMethodExists(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + + // Shutdown should be callable even without a running server + // It should return nil or an error (not panic) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + // When no http server has been started, Shutdown should return nil gracefully + err = server.Shutdown(ctx) + if err != nil { + t.Errorf("expected nil error from Shutdown when server not started, got: %v", err) + } +} + +func TestShutdownStopsServer(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + + // Bind to loopback on an ephemeral port + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + + // Start serving + errCh := make(chan error, 1) + go func() { + errCh <- server.ServeHTTP(ln) + }() + + // Give the server a moment to start accepting + time.Sleep(50 * time.Millisecond) + + // Shutdown should succeed + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + err = server.Shutdown(ctx) + if err != nil { + t.Errorf("expected nil error from Shutdown, got: %v", err) + } + + // Server should have returned from Serve + select { + case serveErr := <-errCh: + if serveErr != nil && serveErr != http.ErrServerClosed { + t.Errorf("expected ErrServerClosed or nil, got: %v", serveErr) + } + case <-time.After(5 * time.Second): + t.Error("server did not shut down within timeout") + } +} + +func TestReadyFieldIsAtomicBool(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + + // Test that concurrent access is safe (race detector will catch issues) + done := make(chan struct{}) + go func() { + for i := 0; i < 1000; i++ { + server.SetReady() + } + close(done) + }() + + for i := 0; i < 1000; i++ { + server.IsReady() + } + <-done +} + +func TestInFlightRequestsCompleteDuringShutdown(t *testing.T) { + log := logrus.New() + log.SetOutput(io.Discard) + + server, err := NewWebhookServer(log, &mockBackend{}) + if err != nil { + t.Fatalf("failed to create webhook server: %v", err) + } + server.SetReady() + + // Bind to loopback on an ephemeral port + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + addr := ln.Addr().String() + + // Start serving + go func() { + _ = server.ServeHTTP(ln) + }() + + // Give the server a moment to start accepting + time.Sleep(50 * time.Millisecond) + + // Start a long-running request to /healthz (simulates in-flight request) + var wg sync.WaitGroup + wg.Add(1) + requestCompleted := make(chan struct{}) + go func() { + defer wg.Done() + resp, err := http.Get(fmt.Sprintf("http://%s/healthz", addr)) + if err != nil { + t.Errorf("in-flight request failed: %v", err) + return + } + resp.Body.Close() + close(requestCompleted) + }() + + // Wait for the request to complete before shutting down + // (healthz is fast, so it completes immediately) + <-requestCompleted + + // Now start another request and initiate shutdown simultaneously + wg.Add(1) + go func() { + defer wg.Done() + // This request should complete because shutdown waits for in-flight requests + resp, err := http.Get(fmt.Sprintf("http://%s/healthz", addr)) + if err != nil { + // Connection may be refused after shutdown starts - this is acceptable + return + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("expected 200 from in-flight request, got %d", resp.StatusCode) + } + }() + + // Initiate graceful shutdown + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + err = server.Shutdown(ctx) + if err != nil { + t.Errorf("shutdown failed: %v", err) + } + + wg.Wait() +} From b4d91faa2014b6d2ea098fcb93e9881d5125f0a0 Mon Sep 17 00:00:00 2001 From: License Bot Date: Thu, 11 Jun 2026 13:47:28 +0000 Subject: [PATCH 2/2] Automatic application of license header --- ccrn-chart/templates/crds/rhel_storage/server.yaml | 3 +++ ccrn-chart/templates/crds/rhel_storage/volume.yaml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ccrn-chart/templates/crds/rhel_storage/server.yaml b/ccrn-chart/templates/crds/rhel_storage/server.yaml index c33d545..b978789 100644 --- a/ccrn-chart/templates/crds/rhel_storage/server.yaml +++ b/ccrn-chart/templates/crds/rhel_storage/server.yaml @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +# SPDX-License-Identifier: Apache-2.0 + # SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company # SPDX-License-Identifier: Apache-2.0 diff --git a/ccrn-chart/templates/crds/rhel_storage/volume.yaml b/ccrn-chart/templates/crds/rhel_storage/volume.yaml index 6e784e6..a203ceb 100644 --- a/ccrn-chart/templates/crds/rhel_storage/volume.yaml +++ b/ccrn-chart/templates/crds/rhel_storage/volume.yaml @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Greenhouse contributors +# SPDX-License-Identifier: Apache-2.0 + # SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company # SPDX-License-Identifier: Apache-2.0