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
2 changes: 1 addition & 1 deletion .vale/styles/HouseStyle/Headings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ exceptions:
- GitProvider
- WatchRule
- ClusterWatchRule
- ClusterProvider
- KubeProvider
- CommitRequest
- APIResourceCatalog
- HelmRelease
Expand Down
4 changes: 2 additions & 2 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ resources:
version: v1alpha3
- api:
crdVersion: v1
namespaced: false
namespaced: true
controller: true
domain: configbutler.ai
kind: ClusterProvider
kind: KubeProvider
path: github.com/ConfigButler/gitops-reverser/api/v1alpha3
version: v1alpha3
version: "3"
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ agent over MCP.

Custom resources configure it: `GitProvider` for repository and credentials, `GitBranch` for one
branch of that repository, `GitTarget` for a folder on it, `WatchRule` or `ClusterWatchRule` for what
to capture, and `ClusterProvider` for the source cluster, created as `default` on install. A rule
selects whole resource collections, including their deletions; the `GitTarget`'s prune mode decides
whether a deletion removes the file from Git.
to capture, and `KubeProvider` for the source cluster, created as `local` in the operator's
namespace on install. A rule selects whole resource collections, including their deletions; the
`GitTarget`'s prune mode decides whether a deletion removes the file from Git.
See [Configuration](docs/configuration.md).

By default it watches its own cluster. Add more of each as you need them: one operator can serve
Expand Down
4 changes: 2 additions & 2 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ For what the controller can access and which pieces are sensitive, see
## Shared audit-ingress trust model

Audit ingress uses mutual TLS, but the current multi-cluster routes use the client certificate to
authenticate membership in the shared audit CA—not to bind a sender to one `ClusterProvider`. A
authenticate membership in the shared audit CA, not to bind a sender to one `KubeProvider`. A
source that holds the shared client credential can therefore submit audit facts for any configured
provider, whether the provider is selected by `/audit-webhook/<provider>` or by shared-stream
provider, whether the provider is selected by `/audit-webhook/<route>` or by shared-stream
annotation routing.

This is an explicit, accepted operating assumption for now: use shared ingress only when a highly
Expand Down
6 changes: 3 additions & 3 deletions Taskfile-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -756,10 +756,10 @@ tasks:
cmds:
- |
mkdir -p dist
# CRDs ship as their OWN file, applied FIRST. The bundle contains the reserved `default`
# ClusterProvider — a custom resource that cannot be applied in the same `kubectl apply` as
# CRDs ship as their OWN file, applied FIRST. The bundle contains the `local`
# KubeProvider — a custom resource that cannot be applied in the same `kubectl apply` as
# the CRD defining it: kubectl builds its RESTMapper up front and fails with
# `no matches for kind "ClusterProvider"`. So: kubectl apply -f dist/crds.yaml, then
# `no matches for kind "KubeProvider"`. So: kubectl apply -f dist/crds.yaml, then
# kubectl apply -f dist/install.yaml. `helm install` is unaffected — it installs
# charts/gitops-reverser/crds/ in its own first phase, which is why --include-crds is gone
# here. Each chart CRD file already begins with its own `---`, so a plain cat is valid
Expand Down
18 changes: 9 additions & 9 deletions api/v1alpha3/applytrigger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const (

// Why a kind and a name, and no namespace: the kind keeps each trigger its own schema, the way
// Flux's sourceRef names a kind, and the namespace is the GitTarget's own because the trigger's
// ClusterProvider admits the namespace it is in (spec.accessFrom). A GitTarget that could name a
// KubeProvider admits the namespace it is in. A GitTarget that could name a
// trigger elsewhere would drive an applier that its own namespace was never admitted to.

// ApplyTriggerReference names an apply trigger in the GitTarget's own namespace.
Expand Down Expand Up @@ -151,8 +151,8 @@ type ApplyTrigger interface {

// TriggerKind is the trigger's kind (ApplyTriggerKind*).
TriggerKind() string
// ClusterProvider is the name of the ClusterProvider whose cluster holds the applier.
ClusterProvider() string
// KubeProviderKey is the KubeProvider whose cluster holds the applier.
KubeProviderKey() types.NamespacedName
// ApplierKind is the kind of applier the trigger holds (ApplierKind*).
ApplierKind() string
// Applier is the object the trigger holds, its namespace defaulted to the trigger's own.
Expand Down Expand Up @@ -193,11 +193,11 @@ func defaultedNamespace(namespace, fallback string) string {
return namespace
}

// clusterProviderName is ref's name, or DefaultClusterProviderName when ref names none, the way
// GitTarget.SourceCluster resolves an omitted clusterProviderRef.
func clusterProviderName(ref *meta.LocalObjectReference) string {
if ref == nil || ref.Name == "" {
return DefaultClusterProviderName
// kubeProviderKey is ref, its namespace defaulted to the referrer's own, or the zero key when ref
// names nothing.
func kubeProviderKey(ref meta.NamespacedObjectReference, referrerNamespace string) types.NamespacedName {
if ref.Name == "" {
return types.NamespacedName{}
}
return ref.Name
return types.NamespacedName{Namespace: defaultedNamespace(ref.Namespace, referrerNamespace), Name: ref.Name}
}
37 changes: 22 additions & 15 deletions api/v1alpha3/argocdtrigger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,28 @@ import (
"k8s.io/apimachinery/pkg/types"
)

// spec.clusterProviderRef was replaced by spec.kubeProviderRef, with the cluster-scoped kind it
// named. It stays in the schema only to be refused by name; the rule sits on spec so CRD validation
// ratcheting cannot wave a stored value through on the next edit.

// ArgoCDTriggerSpec names the Argo CD Application the trigger holds, and the cluster it is in.
// +kubebuilder:validation:XValidation:rule="!has(self.clusterProviderRef)",message="spec.clusterProviderRef was replaced by spec.kubeProviderRef, which names a namespaced KubeProvider"
type ArgoCDTriggerSpec struct {
// Immutable for the reason FluxTriggerSpec's references are: the trigger holds what it names.

// ClusterProviderRef names the ClusterProvider whose cluster holds the Application, such as a
// management cluster running Argo CD for the source cluster. A provider without
// spec.kubeConfig is the operator's own cluster. The provider must admit this trigger's
// namespace (spec.accessFrom). Its identity needs get and update on the Application. Omitted,
// it is the ClusterProvider named "default", as for a GitTarget. Immutable.
// KubeProviderRef names the KubeProvider whose cluster holds the Application, such as a
// management cluster running Argo CD for the source cluster. Namespace omitted means this
// trigger's own namespace. The provider must admit this trigger's namespace: its own, or one its
// spec.accessFrom admits. Its identity needs get and update on the Application. Immutable.
// +required
// +kubebuilder:validation:XValidation:rule="self.name != ''",message="spec.kubeProviderRef.name must not be empty"
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.kubeProviderRef is immutable; delete and recreate the ArgoCDTrigger to hold an Application elsewhere"
KubeProviderRef meta.NamespacedObjectReference `json:"kubeProviderRef"`

// RemovedClusterProviderRef is the replaced spelling of kubeProviderRef. It is never read;
// setting it is refused.
// +optional
// +kubebuilder:default={name: "default"}
// +kubebuilder:validation:XValidation:rule="self.name != ''",message="spec.clusterProviderRef.name must not be empty"
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.clusterProviderRef is immutable; delete and recreate the ArgoCDTrigger to hold an Application elsewhere"
ClusterProviderRef *meta.LocalObjectReference `json:"clusterProviderRef,omitempty"`
RemovedClusterProviderRef *meta.LocalObjectReference `json:"clusterProviderRef,omitempty"`

// ApplicationRef is the Argo CD Application that applies the folders of the GitTargets naming
// this trigger. It must have a single spec.source whose targetRevision names a branch, and
Expand All @@ -48,7 +56,7 @@ type ArgoCDTriggerStatus struct {
// syncs a single source, has automated sync off, may be updated, and no other ArgoCDTrigger
// holds it) and the kstatus Ready/Reconciling/Stalled trio. A trigger acts only while an
// eligible GitTarget references it: one in its namespace that names it, is admitted by its
// ClusterProviders, and writes the repository and branch the Application syncs. A trigger no
// KubeProviders, and writes the repository and branch the Application syncs. A trigger no
// GitTarget names reads nothing in the Application's cluster and is Configured=Unknown,
// Ready=True, both NotReferenced; one named only by GitTargets that are not eligible is
// Configured NotBound or a refusal. Of the ArgoCDTriggers in line for one Application in one cluster, the one
Expand Down Expand Up @@ -98,7 +106,7 @@ type ArgoCDTriggerSource struct {
// +kubebuilder:printcolumn:name="Application",type=string,JSONPath=`.spec.applicationRef.name`
// +kubebuilder:printcolumn:name="Operation",type=string,JSONPath=`.status.operation.phase`
// +kubebuilder:printcolumn:name="Commit",type=string,JSONPath=`.status.operation.commit`,priority=1
// +kubebuilder:printcolumn:name="Cluster",type=string,JSONPath=`.spec.clusterProviderRef.name`,priority=1
// +kubebuilder:printcolumn:name="Cluster",type=string,JSONPath=`.spec.kubeProviderRef.name`,priority=1
// +kubebuilder:printcolumn:name="RepairPending",type=boolean,JSONPath=`.status.repairPending`,priority=1
// +kubebuilder:printcolumn:name="Message",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`,priority=1
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
Expand Down Expand Up @@ -142,10 +150,9 @@ var _ ApplyTrigger = (*ArgoCDTrigger)(nil)
// TriggerKind is ArgoCDTrigger.
func (t *ArgoCDTrigger) TriggerKind() string { return ApplyTriggerKindArgoCD }

// ClusterProvider is spec.clusterProviderRef.name, or "default" when it is unset, as the schema
// defaults it: a literal built in a test or by an older client still names a concrete provider.
func (t *ArgoCDTrigger) ClusterProvider() string {
return clusterProviderName(t.Spec.ClusterProviderRef)
// KubeProviderKey is spec.kubeProviderRef, its namespace defaulted to the trigger's own.
func (t *ArgoCDTrigger) KubeProviderKey() types.NamespacedName {
return kubeProviderKey(t.Spec.KubeProviderRef, t.Namespace)
}

// ApplierKind is Application.
Expand Down
77 changes: 23 additions & 54 deletions api/v1alpha3/audit_route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,61 +9,26 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// TestClusterProvider_AuditRoute covers the defaulting rule the whole feature rests on: an unset
// auditRoute resolves to the provider's own name, which is what every install already partitions its
// facts by, so adding the field changes nothing until someone sets it.
func TestClusterProvider_AuditRoute(t *testing.T) {
tests := []struct {
name string
provider ClusterProvider
want string
}{
{
name: "no attribution block falls back to the object name",
provider: ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "prod-eu-1"}},
want: "prod-eu-1",
},
{
name: "an empty auditRoute falls back to the object name",
provider: ClusterProvider{
ObjectMeta: metav1.ObjectMeta{Name: "prod-eu-1"},
Spec: ClusterProviderSpec{Attribution: &ClusterProviderAttribution{}},
},
want: "prod-eu-1",
},
{
name: "a declared route wins over the object name",
provider: ClusterProvider{
ObjectMeta: metav1.ObjectMeta{Name: "tenant-acme-delegating"},
Spec: ClusterProviderSpec{
Attribution: &ClusterProviderAttribution{AuditRoute: "prod-eu-1"},
},
},
want: "prod-eu-1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.provider.AuditRoute())
})
}
}

// TestClusterProvider_AuditRoute_SharedBySeveralProviders is the reported bug, stated as an API
// TestKubeProvider_AuditRoute_SharedBySeveralProviders is the reported bug, stated as an API
// invariant. An API server has one audit webhook backend and posts under one route, so two
// ClusterProviders naming one physical cluster can only both resolve authors if they agree on that
// route. Before this field they could not: each read under its own name, and every commit through
// the provider the API server did NOT post under was authored attribution-unresolved.
// KubeProviders naming one physical cluster can only both resolve authors if they agree on that
// route. Each defaults to its own <namespace>.<name>, so without a declared route every commit
// through the provider the API server did NOT post under is authored attribution-unresolved.
//
// Locality is deliberately not involved. Both providers here omit kubeConfig, which is what makes
// them in-cluster, and it is the declared route rather than that fact which joins them.
func TestClusterProvider_AuditRoute_SharedBySeveralProviders(t *testing.T) {
fedByTheAPIServer := ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
delegating := ClusterProvider{
ObjectMeta: metav1.ObjectMeta{Name: "srcns-delegating"},
Spec: ClusterProviderSpec{
AllowCrossNamespace: &ClusterProviderCrossNamespace{Sources: true},
Attribution: &ClusterProviderAttribution{AuditRoute: "default"},
// Locality is deliberately not involved. Both providers here set spec.inCluster, and it is the
// declared route rather than that fact which joins them. (The default route defaulting itself is
// TestKubeProvider_AuditRoute.)
func TestKubeProvider_AuditRoute_SharedBySeveralProviders(t *testing.T) {
fedByTheAPIServer := KubeProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: "platform", Name: "local"},
Spec: KubeProviderSpec{InCluster: &KubeProviderInCluster{}},
}
delegating := KubeProvider{
ObjectMeta: metav1.ObjectMeta{Namespace: "platform", Name: "srcns-delegating"},
Spec: KubeProviderSpec{
InCluster: &KubeProviderInCluster{},
AllowCrossNamespace: &KubeProviderCrossNamespace{Sources: true},
Attribution: &KubeProviderAttribution{AuditRoute: "platform.local"},
},
}

Expand All @@ -76,6 +41,10 @@ func TestClusterProvider_AuditRoute_SharedBySeveralProviders(t *testing.T) {
// cross-credit. This is what a declared route buys that keying by object UID alone would not:
// an etcd-snapshot clone reproduces object UIDs exactly, and only a human-chosen name separates
// the copy from its origin.
clone := ClusterProvider{ObjectMeta: metav1.ObjectMeta{Name: "prod-restored"}}
clone := KubeProvider{ObjectMeta: metav1.ObjectMeta{Namespace: "platform", Name: "prod-restored"}}
assert.NotEqual(t, fedByTheAPIServer.AuditRoute(), clone.AuditRoute())

// One provider name in another namespace is another partition by default.
sameName := KubeProvider{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant-a", Name: "local"}}
assert.NotEqual(t, fedByTheAPIServer.AuditRoute(), sameName.AuditRoute())
}
Loading
Loading