Skip to content

Switch ACK to CEL-Based Immutability and Remove Runtime Checks #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/config/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,9 @@ type FieldConfig struct {
// IsSecret instructs the code generator that this field should be a
// SecretKeyReference.
IsSecret bool `json:"is_secret"`
// IsImmutable instructs the code generator to add advisory conditions
// if user modifies the spec field after resource was created.
// IsImmutable indicates that the field is enforced as immutable at the
// admission layer. The code generator will add kubebuilder:validation:XValidation
// lines to the CRD, preventing changes to this field after it’s set.
IsImmutable bool `json:"is_immutable"`
// From instructs the code generator that the value of the field should
// be retrieved from the specified operation and member path
Expand Down
27 changes: 0 additions & 27 deletions pkg/model/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,33 +336,6 @@ func (r *CRD) IsSecretField(path string) bool {
return false
}

// GetImmutableFieldPaths returns list of immutable field paths present in CRD
func (r *CRD) GetImmutableFieldPaths() []string {
fConfigs := r.cfg.GetFieldConfigs(r.Names.Original)
var immutableFields []string

for field, fieldConfig := range fConfigs {
if fieldConfig.IsImmutable {
immutableFields = append(immutableFields, field)
}
}

// We need a deterministic order to traverse the immutable fields
sort.Strings(immutableFields)
return immutableFields
}

// HasImmutableFieldChanges helper function that return true if there are any immutable field changes
func (r *CRD) HasImmutableFieldChanges() bool {
fConfigs := r.cfg.GetFieldConfigs(r.Names.Original)
for _, fieldConfig := range fConfigs {
if fieldConfig.IsImmutable {
return true
}
}
return false
}

// OmitUnchangedFieldsOnUpdate returns whether the controller needs to omit
// unchanged fields from an update request or not.
func (r *CRD) OmitUnchangedFieldsOnUpdate() bool {
Expand Down
12 changes: 12 additions & 0 deletions pkg/model/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,18 @@ func (f *Field) IsRequired() bool {
)
}

// IsImmutable checks the FieldConfig for the Field and returns true if the
// field is marked as immutable.
//
// If the FieldConfig is nil or IsImmutable is false (or not set), this function
// returns false.
func (f *Field) IsImmutable() bool {
if f.FieldConfig != nil && f.FieldConfig.IsImmutable {
return true
}
return false
}

// GetSetterConfig returns the SetFieldConfig object associated with this field
// and a supplied operation type, or nil if none exists.
func (f *Field) GetSetterConfig(opType OpType) *ackgenconfig.SetFieldConfig {
Expand Down
14 changes: 10 additions & 4 deletions templates/apis/crd.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,15 @@ type {{ .CRD.Kind }}Spec struct {
{{ if $field.GetDocumentation -}}
{{ $field.GetDocumentation }}
{{ end -}}
{{- if and ($field.IsRequired) (not $field.HasReference) -}}

{{- if $field.IsImmutable }}
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable once set"
{{- end }}

{{- if and ($field.IsRequired) (not $field.HasReference) }}
// +kubebuilder:validation:Required
{{ end -}}
{{- end }}

{{ $field.Names.Camel }} {{ $field.GoType }} {{ $field.GetGoTag }}
{{- end }}
}
Expand All @@ -33,7 +39,7 @@ type {{ .CRD.Kind }}Status struct {
// constructed ARN for the resource
// +kubebuilder:validation:Optional
ACKResourceMetadata *ackv1alpha1.ResourceMetadata `json:"ackResourceMetadata"`
// All CRS managed by ACK have a common `Status.Conditions` member that
// All CRs managed by ACK have a common `Status.Conditions` member that
// contains a collection of `ackv1alpha1.Condition` objects that describe
// the various terminal states of the CR and its backend AWS service API
// resource
Expand Down Expand Up @@ -80,4 +86,4 @@ type {{ .CRD.Kind }}List struct {

func init() {
SchemeBuilder.Register(&{{ .CRD.Kind }}{}, &{{ .CRD.Kind }}List{})
}
}
3 changes: 1 addition & 2 deletions templates/crossplane/apis/crd.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,4 @@ var (

func init() {
SchemeBuilder.Register(&{{ .CRD.Kind }}{}, &{{ .CRD.Kind }}List{})
}

}
17 changes: 0 additions & 17 deletions templates/pkg/resource/sdk.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -328,23 +328,6 @@ func (rm *resourceManager) terminalAWSError(err error) bool {
{{- end }}
}

{{- if .CRD.HasImmutableFieldChanges }}
// getImmutableFieldChanges returns list of immutable fields from the
func (rm *resourceManager) getImmutableFieldChanges(
delta *ackcompare.Delta,
) []string {
var fields []string;
{{- $prefixConfig := .CRD.Config.PrefixConfig }}
{{- range $immutableField := .CRD.GetImmutableFieldPaths }}
{{- $specPrefix := $prefixConfig.SpecField }}
if delta.DifferentAt("{{ TrimPrefix $specPrefix "." }}.{{$immutableField}}") {
fields = append(fields,"{{$immutableField}}")
}
{{- end }}

return fields
}
{{- end }}
{{- if $hookCode := Hook .CRD "sdk_file_end" }}
{{ $hookCode }}
{{- end }}
6 changes: 0 additions & 6 deletions templates/pkg/resource/sdk_update.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ func (rm *resourceManager) sdkUpdate(
defer func() {
exit(err)
}()
{{- if .CRD.HasImmutableFieldChanges }}
if immutableFieldChanges := rm.getImmutableFieldChanges(delta); len(immutableFieldChanges) > 0 {
msg := fmt.Sprintf("Immutable Spec fields have been modified: %s", strings.Join(immutableFieldChanges, ","))
return nil, ackerr.NewTerminalError(fmt.Errorf(msg))
}
{{- end }}
{{- if $hookCode := Hook .CRD "sdk_update_pre_build_request" }}
{{ $hookCode }}
{{- end }}
Expand Down