Skip to content

Default ApplicationLayer and ManagementCluster fields in the CRD schema - #5241

Open
caseydavenport wants to merge 1 commit into
tigera:masterfrom
caseydavenport:casey-crd-schema-defaults
Open

Default ApplicationLayer and ManagementCluster fields in the CRD schema#5241
caseydavenport wants to merge 1 commit into
tigera:masterfrom
caseydavenport:casey-crd-schema-defaults

Conversation

@caseydavenport

Copy link
Copy Markdown
Member

Description

Follow-on to #5203. The other CRs have the same problem: the operator defaults a field, patches it back, and becomes the field manager, so a server-side apply from Helm or Argo conflicts. Installation had to record its defaults in the status because its defaulting needs cluster state and because of the overlay, and neither applies here, so these are plain CRD schema defaults.

This covers ApplicationLayer and ManagementCluster, the two CRs where schema defaults replace the write-back completely:

  • every value matches what the controller was defaulting, so an object that already has the value stored is unaffected
  • both controllers keep defaulting in memory, which still covers a cluster whose CRDs the operator doesn't manage
  • the spec write-back is gone from both, which is what releases field ownership

One behavior change, in ApplicationLayer: the L7 log interval and requests-per-interval were only defaulted when log collection was enabled, and they now always default. Nothing renders them unless log collection is on, so the only difference is the two fields appearing in the spec.

Increments left for follow-ups, all for a reason rather than for size:

  • Monitor: the external Prometheus scrape parameters are a map of string to list, and controller-gen emits the default as a string rather than a list, so the schema default would be invalid
  • ManagementClusterConnection: the tunnel CA is fine, but the Enterprise extension also defaults the impersonation permissions in the spec, and that field distinguishes unset from empty
  • LogCollector: process path collection and syslog encryption are fine, but the syslog log types default only exists to fill in objects created before that field, and it is a required field
  • Authentication: the OIDC email verification and LDAP name attribute are fine, but the username and groups prefixes are copied from another field, which a schema default can't do

Each of those keeps its write-back until the whole CR can drop it, since a schema default that doesn't let us remove the patch doesn't release ownership.

Related: #5102

Release Note

Fixes server-side apply conflicts on ApplicationLayer and ManagementCluster fields between the operator and the tool that manages the resource, such as Helm or Argo CD.

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

The API server applies these defaults, so the operator no longer takes
field ownership by writing them into the spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants