Skip to content
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
61 changes: 61 additions & 0 deletions api/v1alpha3/gittarget_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,20 @@ type GitTargetSpec struct {
// objects have no namespace. Full resolution table: docs/configuration.md.
// +optional
AllowedSourceNamespaces *NamespaceMatcher `json:"allowedSourceNamespaces,omitempty"`

// Design rationale, kept out of the generated CRD description by the blank line below.
//
// Deliberately MUTABLE, unlike the destination fields above. The whole point of the safe
// default is that a target keeps its documents while a scope mistake is diagnosed; turning
// convergence back on afterwards must not require deleting and recreating the GitTarget, which
// would be the one operation guaranteed to lose the folder's history.

// Prune controls which deletion paths may remove documents from this target's folder: an
// explicit source DELETE event, and the resync mark-and-sweep that infers a deletion from a
// desired snapshot. Omitted, it is `mode: OnEvent` — observed deletes are mirrored, inferred
// ones are not — for a stored GitTarget as well as a new one.
// +optional
Prune *PrunePolicy `json:"prune,omitempty"`
}

// GitTargetPlacementSpec declares where NEW resources are written when no document
Expand Down Expand Up @@ -193,6 +207,22 @@ type GitTargetStatus struct {
// Counts, never a per-type list, so it stays bounded however many types are watched.
// +optional
Streams *GitTargetStreamsStatus `json:"streams,omitempty"`

// Design rationale, kept out of the generated CRD description by the blank line below.
//
// An observation, not a condition. A sweep suppressed by spec.prune.mode is the configured
// outcome and a healthy reconciliation, so no condition may go False for it — doing so would
// train operators to ignore the conditions that mean the mirror is genuinely broken. The
// distinction this field rests on: a condition asserts health, an observation reports a fact.
// status.streams is the precedent for the second kind.

// Retention reports documents a resync kept because this target's spec.prune.mode suppressed
// the mark-and-sweep. It covers the INFERRED deletion path only: under `never`, a suppressed
// source DELETE is not counted here, so a `never` target can report zero while still declining
// to mirror deletes. It is informational either way — retention is the configured behavior,
// never a fault, and no condition changes state because of it.
// +optional
Retention *GitTargetRetentionStatus `json:"retention,omitempty"`
}

// GitTargetStreamsStatus is a bounded roll-up of the stream readiness state for the
Expand All @@ -219,6 +249,37 @@ type GitTargetStreamsStatus struct {
ObservedTime *metav1.Time `json:"observedTime,omitempty"`
}

// Design rationale, kept out of the generated CRD description by the blank line below.
//
// Counts, never a per-document list, for the same reason GitTargetStreamsStatus is counts: the
// field must stay bounded however many documents are retained. An operator who needs to know WHICH
// documents reads the retention log line or the folder; status answers "how many, under what
// policy, as of when".
//
// The projection is pull-based — the GitTarget controller reads it from the watch manager on each
// reconcile — so it is only as fresh as the last reconcile, exactly like GitPathAccepted. That is
// acceptable for an observation and would not be for a gate, which is a further reason this must
// not become a condition.

// GitTargetRetentionStatus is a bounded roll-up of what this GitTarget's prune policy kept.
type GitTargetRetentionStatus struct {
// Mode is the EFFECTIVE spec.prune.mode this roll-up was produced under. It is reported here
// rather than left to be read from the spec because a GitTarget that predates spec.prune has
// no stored value at all, so the spec alone cannot explain why documents are being kept.
// +optional
Mode PruneMode `json:"mode,omitempty"`

// RetainedDocuments is how many managed documents the policy kept that a converged mirror
// would not hold. Zero means a resync ran and found nothing to retain — the mirror is
// converged. An ABSENT retention block means something different: no resync has reported yet.
RetainedDocuments int32 `json:"retainedDocuments"`

// ObservedTime is when this roll-up was last computed. A retention that begins just after a
// reconcile is not visible until the next one, so read this before treating a zero as live.
// +optional
ObservedTime *metav1.Time `json:"observedTime,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Provider",type=string,JSONPath=`.spec.providerRef.name`
Expand Down
142 changes: 142 additions & 0 deletions api/v1alpha3/prune_policy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// SPDX-License-Identifier: Apache-2.0

package v1alpha3

// PruneMode enumerates which of the two deletion paths may remove a managed document from a
// GitTarget's folder.
//
// | source DELETE event | resync mark-and-sweep
// Never | suppressed | suppressed
// OnEvent | applied | suppressed
// Always | applied | applied
type PruneMode string

const (
// PruneNever removes nothing: neither an explicit source DELETE nor an inferred sweep drop.
// The folder becomes an archive that only ever gains and updates documents.
PruneNever PruneMode = "Never"
// PruneOnEvent mirrors an observed source DELETE but never infers a deletion from a desired
// snapshot. It is the effective default.
PruneOnEvent PruneMode = "OnEvent"
// PruneAlways enables both paths: full desired-state convergence, including removing a Git
// document whose resource is absent from the snapshot.
PruneAlways PruneMode = "Always"
)

// Design rationale, kept out of the generated CRD description by the blank line below.
//
// An object rather than a bare enum field on GitTargetSpec, so a later volume guard (for example
// maxDeletesPerCommit) can be added as a sibling field. Shipping the enum as a scalar would force
// a scalar-to-object change later, which is breaking; shipping the object costs one nesting level
// now and nothing afterwards.

// PrunePolicy declares which deletion paths may remove documents from a GitTarget's folder.
type PrunePolicy struct {
// Design rationale, kept out of the generated CRD description by the blank line below.
//
// The kubebuilder default writes OnEvent into a NEWLY created object, which is useful but is
// deliberately NOT the compatibility mechanism: a GitTarget stored before this field existed
// carries no value at all, and Kubernetes does not retro-default stored objects. Every reader
// must therefore go through EffectivePruneMode, which maps both an absent policy and an empty
// mode to OnEvent — so an old GitTarget becomes safe without first being edited.

// Mode selects which deletion paths are enabled. `Never` removes nothing; `OnEvent` mirrors an
// observed source DELETE but never infers a deletion from a resync snapshot; `Always` enables
// both, restoring full desired-state convergence. Omitted, it is `OnEvent`.
// +optional
// +kubebuilder:validation:Enum=Never;OnEvent;Always
// +kubebuilder:default=OnEvent
Mode PruneMode `json:"mode,omitempty"`
}

// EffectiveMode resolves the declared mode to the one the controller acts on. A nil policy (the
// field was never written) and an empty mode (written without a mode, or stored before the schema
// default existed) both resolve to PruneOnEvent, so an unedited legacy GitTarget is safe.
//
// The nil receiver is deliberate: it makes the omitted case answerable without every call site
// repeating a nil check, which is where a "safe unless someone forgot" default goes wrong.
func (p *PrunePolicy) EffectiveMode() PruneMode {
if p == nil {
return PruneOnEvent
}
return p.Mode.OrDefault()
}

// OrDefault resolves the EMPTY mode — unset, which is not a mode — to the documented default.
//
// It exists because the empty string is the one value that must not be read literally: both
// predicates below answer false for it, which is `Never`'s behaviour, not `OnEvent`'s. Any value
// that has travelled through a struct literal, a retained pending write, or a stored object
// written before the schema default therefore passes through here first. An UNRECOGNIZED value is
// deliberately left alone — see SweepsOrphans.
func (m PruneMode) OrDefault() PruneMode {
if m == "" {
return PruneOnEvent
}
return m
}

// AppliesEventDeletes reports whether an explicit source DELETE event may remove its managed
// document. True for OnEvent and Always. Call OrDefault first if the value may be unset.
func (m PruneMode) AppliesEventDeletes() bool {
return m == PruneOnEvent || m == PruneAlways
}

// SweepsOrphans reports whether a resync may drop a managed document that its desired snapshot did
// not contain — the inferred mark-and-sweep deletion. True only for Always.
//
// An unrecognized value (an object stored under a schema that allowed more than this build does)
// reads as false here and as false in AppliesEventDeletes: an unknown policy retains everything,
// because the failure mode of guessing wrong in the other direction is deleting a tenant's
// manifests. The empty string is NOT such a value — it is unset, and OrDefault resolves it.
func (m PruneMode) SweepsOrphans() bool {
return m == PruneAlways
}

// EffectivePruneMode is the mode this GitTarget's writes are subject to, with the omitted-field
// default applied. It is the only supported way to read the policy: reading spec.prune.mode
// directly would treat a legacy GitTarget as if it had no mode rather than OnEvent.
func (g *GitTarget) EffectivePruneMode() PruneMode {
return g.Spec.Prune.EffectiveMode()
}

// The modes ordered by how much deletion they authorize. An unrecognized value ranks with Never,
// matching what both predicates already do with it — an unknown policy retains on both paths.
const (
pruneRankNever = iota
pruneRankOnEvent
pruneRankAlways
)

func (m PruneMode) restrictiveness() int {
switch m.OrDefault() {
case PruneAlways:
return pruneRankAlways
case PruneOnEvent:
return pruneRankOnEvent
case PruneNever:
return pruneRankNever
default:
return pruneRankNever
}
}

// MoreRestrictiveOf returns whichever of the two modes authorizes less deletion.
//
// It exists for one situation: a write that was planned under one policy and is applied — or
// replayed after a rebase — under another. Taking the minimum makes the two directions behave the
// way an operator means them, and they are NOT symmetric:
//
// - LOOSENING must not escalate an already-planned write. A resync planned under `OnEvent` chose
// to keep its orphans against a desired snapshot that is now stale; someone declaring `Always`
// afterwards must not turn that stale plan into deletions. The new policy applies to the next
// resync, which gathers a fresh snapshot.
// - TIGHTENING must apply immediately. `Always` -> `OnEvent` is what an operator reaches for to
// stop deletions that have not landed yet; a policy change that queued work could outrun would
// not be a stop button at all.
func (m PruneMode) MoreRestrictiveOf(other PruneMode) PruneMode {
if other.restrictiveness() < m.restrictiveness() {
return other.OrDefault()
}
return m.OrDefault()
}
85 changes: 85 additions & 0 deletions api/v1alpha3/prune_policy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// SPDX-License-Identifier: Apache-2.0

package v1alpha3

import (
"testing"

"github.com/stretchr/testify/assert"
)

// TestEffectivePruneMode_LegacyGitTargetIsOnEvent is the compatibility contract PR 5 rests on.
//
// The CRD default only writes onEvent into a NEWLY created object; Kubernetes does not retro-fill
// stored objects, so a GitTarget written before this field existed reaches the controller with a
// nil spec.prune forever. If that read as anything other than OnEvent, every existing target
// would change behaviour on upgrade without anyone editing it — which is exactly the class of
// surprise this release exists to prevent.
func TestEffectivePruneMode_LegacyGitTargetIsOnEvent(t *testing.T) {
legacy := &GitTarget{}
assert.Equal(t, PruneOnEvent, legacy.EffectivePruneMode(),
"a GitTarget that never declared spec.prune must be OnEvent, not unset")

declaredEmpty := &GitTarget{Spec: GitTargetSpec{Prune: &PrunePolicy{}}}
assert.Equal(t, PruneOnEvent, declaredEmpty.EffectivePruneMode(),
"a prune object written without a mode must also be OnEvent")
}

// TestEffectivePruneMode_HonoursDeclaredMode covers the three declared values end to end through
// the GitTarget accessor, so the helper and the field cannot drift.
func TestEffectivePruneMode_HonoursDeclaredMode(t *testing.T) {
for _, mode := range []PruneMode{PruneNever, PruneOnEvent, PruneAlways} {
target := &GitTarget{Spec: GitTargetSpec{Prune: &PrunePolicy{Mode: mode}}}
assert.Equal(t, mode, target.EffectivePruneMode(), "declared mode %q must survive the accessor", mode)
}
}

// TestPruneMode_PathsMatchTheDocumentedTable pins the two-path table from
// docs/design/watchrule-source-namespace/pr5-gittarget-deletion-safety.md. The two predicates are
// deliberately independent: `onEvent` differs from `always` on ONE of them, and a change that
// collapsed them into a single boolean would silently turn the safe default into full
// convergence.
func TestPruneMode_PathsMatchTheDocumentedTable(t *testing.T) {
for _, tc := range []struct {
mode PruneMode
eventDelete bool
sweep bool
}{
{PruneNever, false, false},
{PruneOnEvent, true, false},
{PruneAlways, true, true},
} {
assert.Equal(t, tc.eventDelete, tc.mode.AppliesEventDeletes(),
"%q: explicit source DELETE", tc.mode)
assert.Equal(t, tc.sweep, tc.mode.SweepsOrphans(),
"%q: resync mark-and-sweep", tc.mode)
}
}

// TestPruneMode_UnsetIsNotNever guards the trap that makes the zero value dangerous: read
// literally, "" answers false to BOTH predicates, which is `never` — a mode that silently stops
// mirroring deletes. Every internal carrier of the value normalizes through OrDefault first.
func TestPruneMode_UnsetIsNotNever(t *testing.T) {
var unset PruneMode

assert.False(t, unset.AppliesEventDeletes(),
"read literally the empty mode looks like never — this is why nothing may read it literally")
assert.Equal(t, PruneOnEvent, unset.OrDefault(),
"OrDefault is the one place that turns unset into the documented default")
assert.True(t, unset.OrDefault().AppliesEventDeletes(),
"a normalized unset mode mirrors an explicit source DELETE")
assert.False(t, unset.OrDefault().SweepsOrphans(),
"a normalized unset mode never infers a deletion")
}

// TestPruneMode_UnrecognizedValueRetains covers the downgrade case: an object stored by a newer
// build under a mode this build's enum does not have. It must fail closed toward RETENTION on
// both paths — never toward deleting content whose policy it cannot understand.
func TestPruneMode_UnrecognizedValueRetains(t *testing.T) {
future := PruneMode("onEventWithApproval")

assert.Equal(t, future, future.OrDefault(),
"an unrecognized value is not unset; OrDefault must leave it alone rather than reinterpret it")
assert.False(t, future.AppliesEventDeletes())
assert.False(t, future.SweepsOrphans())
}
44 changes: 44 additions & 0 deletions api/v1alpha3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cmd/manifest-analyzer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ func runScanFolder(dir, format, policy string, stdout, stderr io.Writer) int {

scanPolicy := manifestanalyzer.ScanPolicy{
Acceptance: manifestanalyzer.AcceptancePolicy{Allowlist: manifestanalyzer.DefaultAllowlist()},
Plan: manifestanalyzer.FolderScanPlanPolicy(),
}
result, err := manifestanalyzer.ScanDir(context.Background(), dir, nil, nil, scanPolicy)
if err != nil {
Expand Down
Loading