Feat: Immutable Parameters (#7059)

Signed-off-by: Brian Kane <briankane1@gmail.com>
This commit is contained in:
Brian Kane
2026-03-19 04:11:22 +00:00
committed by GitHub
parent e5779ec9ec
commit 61e06c0bbb
10 changed files with 895 additions and 5 deletions

View File

@@ -44,6 +44,8 @@ const (
UsageTag = "+usage="
// ShortTag is the short alias annotation
ShortTag = "+short"
// ImmutableTag marks a parameter field as immutable
ImmutableTag = "+immutable"
)
// Template is a helper struct for processing capability including

View File

@@ -159,6 +159,9 @@ const (
// AnnotationAutoUpdate is annotation that let application auto update when it finds definition changes
AnnotationAutoUpdate = "app.oam.dev/autoUpdate"
// AnnotationForceParamMutations bypasses all immutable parameter field validation when set to "true".
AnnotationForceParamMutations = "app.oam.dev/force-param-mutations"
// AnnotationWorkflowName specifies the workflow name for execution.
AnnotationWorkflowName = "app.oam.dev/workflowName"

114
pkg/schema/immutable.go Normal file
View File

@@ -0,0 +1,114 @@
/*
Copyright 2026 The KubeVela Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package schema
import (
"strings"
"cuelang.org/go/cue/ast"
"cuelang.org/go/cue/parser"
"github.com/oam-dev/kubevela/pkg/appfile"
)
// ImmutableFieldsFromTemplate parses a CUE template string and returns the set of
// dotted parameter field paths that are marked with the +immutable comment marker.
func ImmutableFieldsFromTemplate(templateStr string) map[string]bool {
if templateStr == "" {
return nil
}
f, err := parser.ParseFile("-", templateStr, parser.ParseComments)
if err != nil {
return nil
}
paramStruct := getParameterStruct(f)
if paramStruct == nil {
return nil
}
immutableFields := make(map[string]bool)
collectImmutableFields(paramStruct, "", immutableFields)
if len(immutableFields) == 0 {
return nil
}
return immutableFields
}
// collectImmutableFields recursively walks a struct, collecting dotted
// paths of fields marked with +immutable into the result map.
func collectImmutableFields(structLit *ast.StructLit, prefix string, result map[string]bool) {
for _, element := range structLit.Elts {
field, ok := element.(*ast.Field)
if !ok {
continue
}
name := extractFieldName(field)
if name == "" {
continue
}
fieldPath := name
if prefix != "" {
fieldPath = prefix + "." + name
}
if hasImmutableComment(field) {
result[fieldPath] = true
continue
}
if nested, ok := field.Value.(*ast.StructLit); ok {
collectImmutableFields(nested, fieldPath, result)
}
}
}
// getParameterStruct gets the top-level `parameter` struct
func getParameterStruct(f *ast.File) *ast.StructLit {
for _, decl := range f.Decls {
field, ok := decl.(*ast.Field)
if !ok || extractFieldName(field) != "parameter" {
continue
}
if structLit, ok := field.Value.(*ast.StructLit); ok {
return structLit
}
}
return nil
}
// hasImmutableComment reports whether any comment group attached to field
// contains a line matching the +immutable marker.
func hasImmutableComment(field *ast.Field) bool {
for _, commentGroup := range field.Comments() {
for _, comment := range commentGroup.List {
marker := strings.TrimSpace(strings.TrimPrefix(comment.Text, "//"))
if marker == appfile.ImmutableTag {
return true
}
}
}
return false
}
func extractFieldName(field *ast.Field) string {
switch label := field.Label.(type) {
case *ast.Ident:
return label.Name
case *ast.BasicLit:
return strings.Trim(label.Value, `"`)
}
return ""
}

View File

@@ -0,0 +1,138 @@
/*
Copyright 2026 The KubeVela Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package schema
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestImmutableFieldsFromTemplate(t *testing.T) {
cases := []struct {
name string
template string
want map[string]bool
}{
{
name: "empty template",
template: "",
want: nil,
},
{
name: "no parameter block",
template: `
output: {}`,
want: nil,
},
{
name: "parameter block with no immutable fields",
template: `
parameter: {
// +usage=The image to use
image: string
replicas: int
}`,
want: nil,
},
{
name: "parameter with one immutable field",
template: `
parameter: {
// +immutable
image: string
replicas: int
}`,
want: map[string]bool{"image": true},
},
{
name: "multiple immutable fields",
template: `
parameter: {
// +immutable
image: string
// +immutable
storageClass: string
replicas: int
}`,
want: map[string]bool{"image": true, "storageClass": true},
},
{
name: "immutable with usage and short tags on same field",
template: `
parameter: {
// +usage=The image to use
// +short=i
// +immutable
image: string
}`,
want: map[string]bool{"image": true},
},
{
name: "nested immutable field returns dotted path",
template: `
parameter: {
governance: {
// +immutable
tenant: string
region: string
}
image: string
}`,
want: map[string]bool{"governance.tenant": true},
},
{
name: "deeply nested immutable field",
template: `
parameter: {
config: {
db: {
// +immutable
name: string
}
}
}`,
want: map[string]bool{"config.db.name": true},
},
{
name: "mixed top-level and nested immutable fields",
template: `
parameter: {
// +immutable
image: string
governance: {
// +immutable
tenant: string
region: string
}
}`,
want: map[string]bool{"image": true, "governance.tenant": true},
},
{
name: "invalid CUE returns nil",
template: `parameter: { image: }`,
want: nil,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := ImmutableFieldsFromTemplate(tc.template)
assert.Equal(t, tc.want, got)
})
}
}

View File

@@ -48,6 +48,9 @@ context: {
// ErrGenerateOpenAPIV2JSONSchemaForCapability is the error while generating OpenAPI v3 schema
const ErrGenerateOpenAPIV2JSONSchemaForCapability = "cannot generate OpenAPI v3 JSON schema for capability %s: %v"
// ExtensionImmutable is the OpenAPI extension key that marks a parameter field as immutable
const ExtensionImmutable = "x-immutable"
// ParsePropertiesToSchema parse the properties in cue script to the openapi schema
func ParsePropertiesToSchema(ctx context.Context, s string, templateFieldPath ...string) (*openapi3.Schema, error) {
t := s + "\n" + BaseTemplate
@@ -110,6 +113,14 @@ func FixOpenAPISchema(name string, schema *openapi3.Schema) {
}
description := schema.Description
// Extract +immutable first, before other tags may strip the lines containing it.
if cleaned, found := extractMarkerFromDescription(description, appfile.ImmutableTag); found {
description = cleaned
if schema.Extensions == nil {
schema.Extensions = make(map[string]any)
}
schema.Extensions[ExtensionImmutable] = true
}
if strings.Contains(description, appfile.UsageTag) {
description = strings.Split(description, appfile.UsageTag)[1]
}
@@ -119,3 +130,22 @@ func FixOpenAPISchema(name string, schema *openapi3.Schema) {
}
schema.Description = description
}
// extractMarkerFromDescription removes all lines containing marker from description
// and reports whether the marker was found. Used to extract flag-style markers
// like +immutable that have no value component.
func extractMarkerFromDescription(description, marker string) (string, bool) {
found := false
var lines []string
for _, line := range strings.Split(description, "\n") {
if strings.TrimSpace(line) == marker {
found = true
} else {
lines = append(lines, line)
}
}
if !found {
return description, false
}
return strings.TrimSpace(strings.Join(lines, "\n")), true
}

View File

@@ -24,7 +24,9 @@ import (
"github.com/getkin/kin-openapi/openapi3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/oam-dev/kubevela/pkg/appfile"
"github.com/oam-dev/kubevela/pkg/cue/process"
"github.com/oam-dev/kubevela/pkg/oam/util"
)
@@ -196,3 +198,117 @@ func TestConvertOpenAPISchema2SwaggerObject(t *testing.T) {
})
}
}
func TestExtractMarkerFromDescription(t *testing.T) {
cases := []struct {
name string
description string
marker string
wantDescription string
wantFound bool
}{
{
name: "marker on its own line",
description: "some usage\n+immutable\nmore text",
marker: appfile.ImmutableTag,
wantDescription: "some usage\nmore text",
wantFound: true,
},
{
name: "marker not present",
description: "some usage\nmore text",
marker: appfile.ImmutableTag,
wantDescription: "some usage\nmore text",
wantFound: false,
},
{
name: "marker with leading whitespace",
description: "some usage\n +immutable\nmore text",
marker: appfile.ImmutableTag,
wantDescription: "some usage\nmore text",
wantFound: true,
},
{
name: "marker is only content",
description: "+immutable",
marker: appfile.ImmutableTag,
wantDescription: "",
wantFound: true,
},
{
name: "multiple marker lines",
description: "+immutable\nusage text\n+immutable",
marker: appfile.ImmutableTag,
wantDescription: "usage text",
wantFound: true,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, found := extractMarkerFromDescription(tc.description, tc.marker)
assert.Equal(t, tc.wantFound, found)
assert.Equal(t, tc.wantDescription, got)
})
}
}
func TestFixOpenAPISchemaImmutable(t *testing.T) {
cases := []struct {
name string
description string
wantImmutable bool
wantDesc string
}{
{
name: "immutable marker sets extension and cleans description",
description: "+usage=The image to use\n+immutable",
wantImmutable: true,
wantDesc: "The image to use",
},
{
name: "no immutable marker leaves schema unchanged",
description: "+usage=The image to use",
wantImmutable: false,
wantDesc: "The image to use",
},
{
name: "immutable alongside short tag",
description: "+usage=The image to use\n+short=i\n+immutable",
wantImmutable: true,
wantDesc: "The image to use",
},
{
name: "immutable before short tag",
description: "+immutable\n+usage=The image to use\n+short=i",
wantImmutable: true,
wantDesc: "The image to use",
},
{
name: "immutable with no usage tag",
description: "+immutable",
wantImmutable: true,
wantDesc: "",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
schema := &openapi3.Schema{Description: tc.description}
FixOpenAPISchema("field", schema)
assert.Equal(t, tc.wantDesc, schema.Description)
if tc.wantImmutable {
require.NotNil(t, schema.Extensions)
val, ok := schema.Extensions[ExtensionImmutable]
require.True(t, ok, "expected x-immutable extension to be set")
assert.Equal(t, true, val)
} else {
if schema.Extensions != nil {
_, ok := schema.Extensions[ExtensionImmutable]
assert.False(t, ok, "expected x-immutable extension to not be set")
}
}
})
}
}

View File

@@ -0,0 +1,242 @@
/*
Copyright 2026 The KubeVela Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package application
import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"
wfTypesv1alpha1 "github.com/kubevela/pkg/apis/oam/v1alpha1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
"github.com/oam-dev/kubevela/apis/types"
"github.com/oam-dev/kubevela/pkg/appfile"
"github.com/oam-dev/kubevela/pkg/oam"
oamutil "github.com/oam-dev/kubevela/pkg/oam/util"
"github.com/oam-dev/kubevela/pkg/schema"
)
// ValidateImmutableFields validates that immutable parameter fields (`// +immutable`) have not changed
// All checks are bypassed if the app carries the app.oam.dev/force-mutate: "true" annotation.
func (h *ValidatingHandler) ValidateImmutableFields(ctx context.Context, newApp, oldApp *v1beta1.Application) field.ErrorList {
if newApp.Annotations[oam.AnnotationForceParamMutations] == "true" {
return nil
}
defCtx := oamutil.SetNamespaceInCtx(ctx, newApp.Namespace)
var errs field.ErrorList
errs = append(errs, h.validateComponentImmutableFields(defCtx, newApp, oldApp)...)
errs = append(errs, h.validatePolicyImmutableFields(defCtx, newApp, oldApp)...)
errs = append(errs, h.validateWorkflowStepImmutableFields(defCtx, newApp, oldApp)...)
return errs
}
// validateComponentImmutableFields checks immutability for components and their traits.
func (h *ValidatingHandler) validateComponentImmutableFields(ctx context.Context, newApp, oldApp *v1beta1.Application) field.ErrorList {
oldByName := make(map[string]common.ApplicationComponent, len(oldApp.Spec.Components))
for _, c := range oldApp.Spec.Components {
oldByName[c.Name] = c
}
var errs field.ErrorList
for i, newComp := range newApp.Spec.Components {
oldComp, exists := oldByName[newComp.Name]
if !exists || oldComp.Type != newComp.Type {
continue
}
tmpl, err := appfile.LoadTemplate(ctx, h.Client, newComp.Type, types.TypeComponentDefinition, newApp.Annotations)
if err != nil {
klog.V(4).Infof("immutable check: skipping component %q (type %q): %v", newComp.Name, newComp.Type, err)
continue
}
fp := field.NewPath("spec", "components").Index(i).Child("properties")
errs = append(errs, checkImmutableParams(tmpl.TemplateStr, fp, oldComp.Properties, newComp.Properties)...)
errs = append(errs, h.validateTraitImmutableFields(ctx, newApp, i, newComp.Traits, oldComp.Traits)...)
}
return errs
}
// validateTraitImmutableFields checks immutability for the traits on a single component.
func (h *ValidatingHandler) validateTraitImmutableFields(ctx context.Context, app *v1beta1.Application, compIdx int, newTraits, oldTraits []common.ApplicationTrait) field.ErrorList {
var errs field.ErrorList
for j, newTrait := range newTraits {
if j >= len(oldTraits) || oldTraits[j].Type != newTrait.Type {
continue
}
oldTrait := oldTraits[j]
tmpl, err := appfile.LoadTemplate(ctx, h.Client, newTrait.Type, types.TypeTrait, app.Annotations)
if err != nil {
klog.V(4).Infof("immutable check: skipping trait %q on component[%d]: %v", newTrait.Type, compIdx, err)
continue
}
fp := field.NewPath("spec", "components").Index(compIdx).Child("traits").Index(j).Child("properties")
errs = append(errs, checkImmutableParams(tmpl.TemplateStr, fp, oldTrait.Properties, newTrait.Properties)...)
}
return errs
}
// validatePolicyImmutableFields checks immutability for policies.
func (h *ValidatingHandler) validatePolicyImmutableFields(ctx context.Context, newApp, oldApp *v1beta1.Application) field.ErrorList {
oldByName := make(map[string]v1beta1.AppPolicy, len(oldApp.Spec.Policies))
for _, p := range oldApp.Spec.Policies {
oldByName[p.Name] = p
}
var errs field.ErrorList
for i, newPolicy := range newApp.Spec.Policies {
oldPolicy, exists := oldByName[newPolicy.Name]
if !exists || oldPolicy.Type != newPolicy.Type {
continue
}
tmpl, err := appfile.LoadTemplate(ctx, h.Client, newPolicy.Type, types.TypePolicy, newApp.Annotations)
if err != nil {
klog.V(4).Infof("immutable check: skipping policy %q (type %q): %v", newPolicy.Name, newPolicy.Type, err)
continue
}
fp := field.NewPath("spec", "policies").Index(i).Child("properties")
errs = append(errs, checkImmutableParams(tmpl.TemplateStr, fp, oldPolicy.Properties, newPolicy.Properties)...)
}
return errs
}
// validateWorkflowStepImmutableFields checks immutability for workflow steps.
func (h *ValidatingHandler) validateWorkflowStepImmutableFields(ctx context.Context, newApp, oldApp *v1beta1.Application) field.ErrorList {
if newApp.Spec.Workflow == nil || oldApp.Spec.Workflow == nil {
return nil
}
oldByName := indexWorkflowStepsByName(oldApp.Spec.Workflow.Steps)
var errs field.ErrorList
for i, newStep := range newApp.Spec.Workflow.Steps {
oldStep, exists := oldByName[newStep.Name]
if !exists || oldStep.Type != newStep.Type {
continue
}
tmpl, err := appfile.LoadTemplate(ctx, h.Client, newStep.Type, types.TypeWorkflowStep, newApp.Annotations)
if err != nil {
klog.V(4).Infof("immutable check: skipping workflow step %q (type %q): %v", newStep.Name, newStep.Type, err)
continue
}
fp := field.NewPath("spec", "workflow", "steps").Index(i).Child("properties")
errs = append(errs, checkImmutableParams(tmpl.TemplateStr, fp, oldStep.Properties, newStep.Properties)...)
}
return errs
}
// checkImmutableParams extracts immutable field paths from templateStr and verifies
// that those fields have not changed between oldProps and newProps
func checkImmutableParams(templateStr string, fp *field.Path, oldProps, newProps *runtime.RawExtension) field.ErrorList {
immutableFields := schema.ImmutableFieldsFromTemplate(templateStr)
if len(immutableFields) == 0 {
return nil
}
oldMap := rawExtensionToMap(oldProps)
newMap := rawExtensionToMap(newProps)
var errs field.ErrorList
for fieldPath := range immutableFields {
oldVal, oldOK := getNestedValue(oldMap, fieldPath)
newVal, newOK := getNestedValue(newMap, fieldPath)
if !oldOK {
// Field was not set before — first-time population is allowed
continue
}
if !newOK {
errs = append(errs, field.Forbidden(fp.Key(fieldPath),
fmt.Sprintf("immutable field cannot be removed (current: %s)", jsonString(oldVal))))
continue
}
if !jsonEqual(oldVal, newVal) {
errs = append(errs, field.Forbidden(fp.Key(fieldPath),
fmt.Sprintf("immutable field cannot be changed (current: %s, new: %s)", jsonString(oldVal), jsonString(newVal))))
}
}
return errs
}
// getNestedValue retrieves a value from a nested map using a dotted path.
// Returns (value, true) if found, (nil, false) if any segment is missing.
func getNestedValue(m map[string]any, path string) (any, bool) {
parts := strings.SplitN(path, ".", 2)
val, ok := m[parts[0]]
if !ok {
return nil, false
}
if len(parts) == 1 {
return val, true
}
nested, ok := val.(map[string]any)
if !ok {
return nil, false
}
return getNestedValue(nested, parts[1])
}
// rawExtensionToMap decodes a *runtime.RawExtension into a map[string]any.
// Returns nil on nil input or parse error.
func rawExtensionToMap(ext *runtime.RawExtension) map[string]any {
if ext == nil || len(ext.Raw) == 0 {
return nil
}
var m map[string]any
if err := json.Unmarshal(ext.Raw, &m); err != nil {
return nil
}
return m
}
// jsonString returns a compact JSON representation of v, or "<unknown>" on error.
func jsonString(v any) string {
b, err := json.Marshal(v)
if err != nil {
return "<unknown>"
}
return string(b)
}
// jsonEqual reports whether two values are equal by comparing their JSON encodings.
func jsonEqual(a, b any) bool {
aJSON, err := json.Marshal(a)
if err != nil {
return false
}
bJSON, err := json.Marshal(b)
if err != nil {
return false
}
return bytes.Equal(aJSON, bJSON)
}
// indexWorkflowStepsByName returns a map of workflow steps indexed by name.
func indexWorkflowStepsByName(steps []wfTypesv1alpha1.WorkflowStep) map[string]wfTypesv1alpha1.WorkflowStep {
byName := make(map[string]wfTypesv1alpha1.WorkflowStep, len(steps))
for _, step := range steps {
byName[step.Name] = step
}
return byName
}

View File

@@ -0,0 +1,246 @@
/*
Copyright 2026 The KubeVela Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package application
import (
"context"
"encoding/json"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
)
// mustRaw marshals v to a *runtime.RawExtension, panicking on error.
func mustRaw(t *testing.T, v map[string]any) *runtime.RawExtension {
t.Helper()
b, err := json.Marshal(v)
require.NoError(t, err)
return &runtime.RawExtension{Raw: b}
}
func TestCheckImmutableParams(t *testing.T) {
const tmplWithImmutable = `
parameter: {
// +immutable
image: string
replicas: int
}`
fp := field.NewPath("spec", "components").Index(0).Child("properties")
cases := []struct {
name string
template string
oldProps *runtime.RawExtension
newProps *runtime.RawExtension
wantErrCount int
}{
{
name: "no immutable fields in template",
template: `parameter: { image: string }`,
oldProps: mustRaw(t, map[string]any{"image": "v1"}),
newProps: mustRaw(t, map[string]any{"image": "v2"}),
wantErrCount: 0,
},
{
name: "immutable field unchanged",
template: tmplWithImmutable,
oldProps: mustRaw(t, map[string]any{"image": "v1", "replicas": 2}),
newProps: mustRaw(t, map[string]any{"image": "v1", "replicas": 3}),
wantErrCount: 0,
},
{
name: "immutable field changed",
template: tmplWithImmutable,
oldProps: mustRaw(t, map[string]any{"image": "v1"}),
newProps: mustRaw(t, map[string]any{"image": "v2"}),
wantErrCount: 1,
},
{
name: "immutable field removed",
template: tmplWithImmutable,
oldProps: mustRaw(t, map[string]any{"image": "v1"}),
newProps: mustRaw(t, map[string]any{}),
wantErrCount: 1,
},
{
name: "immutable field first set (was absent before)",
template: tmplWithImmutable,
oldProps: mustRaw(t, map[string]any{}),
newProps: mustRaw(t, map[string]any{"image": "v1"}),
wantErrCount: 0,
},
{
name: "nil old props",
template: tmplWithImmutable,
oldProps: nil,
newProps: mustRaw(t, map[string]any{"image": "v1"}),
wantErrCount: 0,
},
{
name: "nested immutable field unchanged",
template: `parameter: { governance: { // +immutable
tenant: string } }`,
oldProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "acme"}}),
newProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "acme"}}),
wantErrCount: 0,
},
{
name: "nested immutable field changed",
template: `
parameter: {
governance: {
// +immutable
tenant: string
region: string
}
}`,
oldProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "acme", "region": "us-east"}}),
newProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "other", "region": "us-east"}}),
wantErrCount: 1,
},
{
name: "nested mutable sibling field can change",
template: `
parameter: {
governance: {
// +immutable
tenant: string
region: string
}
}`,
oldProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "acme", "region": "us-east"}}),
newProps: mustRaw(t, map[string]any{"governance": map[string]any{"tenant": "acme", "region": "eu-west"}}),
wantErrCount: 0,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
errs := checkImmutableParams(tc.template, fp, tc.oldProps, tc.newProps)
assert.Len(t, errs, tc.wantErrCount)
})
}
}
func TestValidateImmutableFields(t *testing.T) {
const immutableTemplate = `
parameter: {
// +immutable
image: string
replicas: int
}`
scheme := runtime.NewScheme()
_ = v1beta1.AddToScheme(scheme)
// Seed a ComponentDefinition with an immutable field in vela-system
compDef := &v1beta1.ComponentDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "webservice",
Namespace: "vela-system",
},
Spec: v1beta1.ComponentDefinitionSpec{
Schematic: &common.Schematic{
CUE: &common.CUE{Template: immutableTemplate},
},
},
}
fakeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(compDef).
Build()
handler := &ValidatingHandler{Client: fakeClient}
ctx := context.Background()
makeApp := func(image string, annotations map[string]string) *v1beta1.Application {
return &v1beta1.Application{
ObjectMeta: metav1.ObjectMeta{
Name: "test-app",
Namespace: "default",
Annotations: annotations,
},
Spec: v1beta1.ApplicationSpec{
Components: []common.ApplicationComponent{
{
Name: "comp1",
Type: "webservice",
Properties: mustRaw(t, map[string]any{"image": image, "replicas": 3}),
},
},
},
}
}
t.Run("immutable field unchanged - no error", func(t *testing.T) {
errs := handler.ValidateImmutableFields(ctx, makeApp("v1", nil), makeApp("v1", nil))
assert.Empty(t, errs)
})
t.Run("immutable field changed - error", func(t *testing.T) {
errs := handler.ValidateImmutableFields(ctx, makeApp("v2", nil), makeApp("v1", nil))
require.Len(t, errs, 1)
assert.Contains(t, errs[0].Detail, `immutable field cannot be changed (current: "v1", new: "v2")`)
})
t.Run("force-param-mutations bypasses all checks", func(t *testing.T) {
errs := handler.ValidateImmutableFields(ctx,
makeApp("v2", map[string]string{"app.oam.dev/force-param-mutations": "true"}),
makeApp("v1", nil),
)
assert.Empty(t, errs)
})
t.Run("unknown definition type - skip gracefully", func(t *testing.T) {
newApp := &v1beta1.Application{
ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"},
Spec: v1beta1.ApplicationSpec{
Components: []common.ApplicationComponent{
{
Name: "comp1",
Type: "unknown-type",
Properties: mustRaw(t, map[string]any{"image": "v2"}),
},
},
},
}
oldApp := &v1beta1.Application{
ObjectMeta: metav1.ObjectMeta{Name: "test-app", Namespace: "default"},
Spec: v1beta1.ApplicationSpec{
Components: []common.ApplicationComponent{
{
Name: "comp1",
Type: "unknown-type",
Properties: mustRaw(t, map[string]any{"image": "v1"}),
},
},
},
}
errs := handler.ValidateImmutableFields(ctx, newApp, oldApp)
assert.Empty(t, errs)
})
}

View File

@@ -56,8 +56,8 @@ func simplifyError(err error) error {
func mergeErrors(errs field.ErrorList) error {
s := ""
for _, err := range errs {
s += fmt.Sprintf("field \"%s\": %s error encountered, %s. ", err.Field, err.Type, err.Detail)
for i, err := range errs {
s += fmt.Sprintf("\n %d) %q: %s.", i+1, err.Field, err.Detail)
}
return errors.New(s)
}

View File

@@ -464,9 +464,8 @@ func (h *ValidatingHandler) ValidateCreate(ctx context.Context, app *v1beta1.App
}
// ValidateUpdate validates the Application on update
func (h *ValidatingHandler) ValidateUpdate(ctx context.Context, newApp, _ *v1beta1.Application, req admission.Request) field.ErrorList {
// check if the newApp is valid
func (h *ValidatingHandler) ValidateUpdate(ctx context.Context, newApp, oldApp *v1beta1.Application, req admission.Request) field.ErrorList {
errs := h.ValidateCreate(ctx, newApp, req)
// TODO: add more validating
errs = append(errs, h.ValidateImmutableFields(ctx, newApp, oldApp)...)
return errs
}