diff --git a/PROJECT b/PROJECT index 933a58b1..6a62ef21 100644 --- a/PROJECT +++ b/PROJECT @@ -49,6 +49,14 @@ resources: kind: ClusterWatchRule path: github.com/ConfigButler/gitops-reverser/api/v1alpha3 version: v1alpha3 +- api: + crdVersion: v1 + namespaced: true + controller: true + domain: configbutler.ai + kind: GitReceiver + path: github.com/ConfigButler/gitops-reverser/api/v1alpha3 + version: v1alpha3 - api: crdVersion: v1 namespaced: true diff --git a/README.md b/README.md index 40c0f4cf..e89e02b5 100644 --- a/README.md +++ b/README.md @@ -48,9 +48,10 @@ 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 `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. +to capture, `KubeProvider` for the source cluster, created as `local` in the operator's namespace on +install, and optionally `GitReceiver` for the Git host's push webhook. 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 diff --git a/api/v1alpha3/examples_test.go b/api/v1alpha3/examples_test.go index ecd60507..fc02f38a 100644 --- a/api/v1alpha3/examples_test.go +++ b/api/v1alpha3/examples_test.go @@ -128,6 +128,8 @@ func newExampleObject(kind string) any { return &GitBranch{} case "GitTarget": return &GitTarget{} + case "GitReceiver": + return &GitReceiver{} case "KubeProvider": return &KubeProvider{} diff --git a/api/v1alpha3/gitbranch_types.go b/api/v1alpha3/gitbranch_types.go index 9864340f..9c2e998c 100644 --- a/api/v1alpha3/gitbranch_types.go +++ b/api/v1alpha3/gitbranch_types.go @@ -77,6 +77,16 @@ type GitBranchStatus struct { // +optional Repository string `json:"repository,omitempty"` + // LastPushNoticeAt is when a GitReceiver last delivered a push to this branch, which made the + // branch worker look at once instead of at its next refresh. It records receipt only: whether + // that look reached the remote is status.remote's to say. Absent means no delivery has reached + // this branch: no GitReceiver is set up, the Git host's webhook does not reach one, the delivery + // names the repository by a URL other than the GitProvider's spec.url, or no branch worker writes + // the branch because no GitTarget references this GitBranch. It advances at most once a minute, + // and is cleared when the GitProvider is recreated for another repository. + // +optional + LastPushNoticeAt *metav1.Time `json:"lastPushNoticeAt,omitempty"` + // Remote is where this branch is on the Git remote, and when that was last proved. It is // renewed by every confirmed look: a successful push as well as a fetch. Absent means nothing // has proved anything yet: nothing has looked, or the GitProvider was recreated against a diff --git a/api/v1alpha3/gitprovider_types.go b/api/v1alpha3/gitprovider_types.go index 0f9c9314..5c4f7f8a 100644 --- a/api/v1alpha3/gitprovider_types.go +++ b/api/v1alpha3/gitprovider_types.go @@ -3,10 +3,6 @@ package v1alpha3 import ( - "crypto/hmac" - "crypto/sha256" - "encoding/hex" - meta "github.com/fluxcd/pkg/apis/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -23,6 +19,13 @@ import ( // commit identity/signing. // // +kubebuilder:validation:XValidation:rule="self.url == oldSelf.url",message="spec.url is immutable; delete and recreate the GitProvider to point at a different repository" +// +// spec.receiver was replaced by the GitReceiver kind, which wakes every GitProvider whose spec.url +// its deliveries name. A pruned field would apply cleanly and leave a provider that used to be +// notified silently unnotified, so the old spelling stays in the schema only to be refused by name. +// The rule sits on spec, not on the field, so CRD validation ratcheting cannot wave a stored value +// through on the next edit. +// +kubebuilder:validation:XValidation:rule="!has(self.receiver)",message="spec.receiver was replaced by a GitReceiver: create one with this type and secretRef, and remove spec.receiver; deliveries wake every GitProvider whose spec.url they name" type GitProviderSpec struct { // URL of the repository (HTTP/SSH). // Immutable: delete and recreate the GitProvider to point at a different repository. @@ -66,68 +69,16 @@ type GitProviderSpec struct { // +optional Commit *CommitSpec `json:"commit,omitempty"` - // The names are Flux's Receiver's, type values included, so a Flux user already knows them. - - // Receiver lets the Git host tell GitOps Reverser that somebody pushed. Point the host's push - // webhook at the receiver listener plus status.webhookPath, with the Secret's token as the - // webhook secret. A delivery is only a hint: it makes GitOps Reverser read where the branch is - // on the remote, and what the remote says decides whether a GitTarget's applier is asked to - // apply. Without a receiver, a foreign push is still found, by the next rejected push or by - // the periodic refresh. + // RemovedReceiver is the old place of a GitReceiver's type and secretRef. It is never read; + // setting it is refused. // +optional - Receiver *GitProviderReceiver `json:"receiver,omitempty"` + RemovedReceiver *RemovedGitProviderReceiver `json:"receiver,omitempty"` } -// GitProviderReceiver is the push-notification endpoint of a GitProvider. -type GitProviderReceiver struct { - // Type is the payload and signature the Git host sends. "github" checks X-Hub-Signature-256, - // "gitlab" checks X-Gitlab-Token, "gitea" checks X-Gitea-Signature, and "generic-hmac" checks - // X-Signature: sha256= over a {"repository", "branch", "after"} body, - // for a relay of your own. - // +required - // +kubebuilder:validation:Enum=github;gitlab;gitea;generic-hmac - Type ReceiverType `json:"type"` - - // SecretRef names a Secret in this namespace whose "token" key is the webhook secret. The - // token also derives status.webhookPath, so rotating it moves the path: register the new path - // with the new secret. - // +required - SecretRef meta.LocalObjectReference `json:"secretRef"` -} - -// ReceiverType is the payload format a receiver accepts. The values are Flux's Receiver types. -type ReceiverType string - -const ( - // ReceiverTypeGitHub is a GitHub push event. - ReceiverTypeGitHub ReceiverType = "github" - // ReceiverTypeGitLab is a GitLab push event. - ReceiverTypeGitLab ReceiverType = "gitlab" - // ReceiverTypeGitea is a Gitea push event. - ReceiverTypeGitea ReceiverType = "gitea" - // ReceiverTypeGenericHMAC is a {"repository", "branch", "after"} body signed as Flux's - // generic-hmac Receiver expects. - ReceiverTypeGenericHMAC ReceiverType = "generic-hmac" -) - -// ReceiverTokenKey is the Secret key holding a receiver's token, the key a Flux Receiver Secret uses. -const ReceiverTokenKey = "token" - -// ReceiverPathPrefix is the prefix every receiver path starts with. -const ReceiverPathPrefix = "/git-push/" - -// A path that names the GitProvider would let anybody who can reach the listener list which -// namespaces and providers exist. Like Flux's Receiver, the path is derived from the token, so -// only whoever holds the token can find it; unlike Flux's sha256(token+name+namespace), it is an -// HMAC over "namespace/name", which no two providers sharing a token can collide on. - -// ReceiverPath is the path a GitProvider's receiver is served on: the prefix and the hex -// HMAC-SHA256 of "/", keyed by the receiver token. -func ReceiverPath(token []byte, namespace, name string) string { - mac := hmac.New(sha256.New, token) - mac.Write([]byte(namespace + "/" + name)) - return ReceiverPathPrefix + hex.EncodeToString(mac.Sum(nil)) -} +// RemovedGitProviderReceiver keeps spec.receiver in the schema so that setting it is refused rather +// than pruned. Any value is kept as written until the refusal names its replacement. +// +kubebuilder:pruning:PreserveUnknownFields +type RemovedGitProviderReceiver struct{} // KnownHostsReference points at a namespace-local ConfigMap or Secret that holds SSH known_hosts // host-trust material. The data is read from the "known_hosts" key, falling back to @@ -223,12 +174,6 @@ type GitProviderStatus struct { // that cadence. Providers are few and the check was already running; this adds no request. // +optional LastVerifiedAt *metav1.Time `json:"lastVerifiedAt,omitempty"` - - // WebhookPath is the path spec.receiver is served on, under the receiver listener: - // /git-push/ and a hash of this provider's name and the receiver token. Empty without - // spec.receiver, and while the token Secret or its "token" key is missing. - // +optional - WebhookPath string `json:"webhookPath,omitempty"` } // CommitSpec configures the commit identity and signing a GitProvider uses. Message formatting diff --git a/api/v1alpha3/gitreceiver_types.go b/api/v1alpha3/gitreceiver_types.go new file mode 100644 index 00000000..d3fc26f1 --- /dev/null +++ b/api/v1alpha3/gitreceiver_types.go @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha3 + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + + meta "github.com/fluxcd/pkg/apis/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// The names are Flux's Receiver's, type values included, so a Flux user already knows them. The +// kind is not called Receiver because Flux's Receiver sits in the same clusters. + +// GitReceiverSpec defines the desired state of GitReceiver. +type GitReceiverSpec struct { + // Type is the payload and signature the Git host sends. "github" checks X-Hub-Signature-256, + // "gitlab" checks X-Gitlab-Token, "gitea" checks X-Gitea-Signature, and "generic-hmac" checks + // X-Signature: sha256= over a {"repository", "branch", "after"} body, + // for a relay of your own. + // +required + // +kubebuilder:validation:Enum=github;gitlab;gitea;generic-hmac + Type ReceiverType `json:"type"` + + // SecretRef names a Secret in this GitReceiver's namespace whose "token" key is the webhook + // secret. The token also derives status.webhookPath, so rotating it moves the path: register the + // new path with the new secret. + // +required + // +kubebuilder:validation:XValidation:rule="self.name != ''",message="spec.secretRef.name must not be empty" + SecretRef meta.LocalObjectReference `json:"secretRef"` +} + +// ReceiverType is the payload format a receiver accepts. The values are Flux's Receiver types. +type ReceiverType string + +const ( + // ReceiverTypeGitHub is a GitHub push event. + ReceiverTypeGitHub ReceiverType = "github" + // ReceiverTypeGitLab is a GitLab push event. + ReceiverTypeGitLab ReceiverType = "gitlab" + // ReceiverTypeGitea is a Gitea push event. + ReceiverTypeGitea ReceiverType = "gitea" + // ReceiverTypeGenericHMAC is a {"repository", "branch", "after"} body signed as Flux's + // generic-hmac Receiver expects. + ReceiverTypeGenericHMAC ReceiverType = "generic-hmac" +) + +// ReceiverTokenKey is the Secret key holding a receiver's token, the key a Flux Receiver Secret uses. +const ReceiverTokenKey = "token" + +// ReceiverPathPrefix is the prefix every receiver path starts with. +const ReceiverPathPrefix = "/git-push/" + +// A path that names the GitReceiver would let anybody who can reach the listener list which +// namespaces and receivers exist. Like Flux's Receiver, the path is derived from the token, so +// only whoever holds the token can find it; unlike Flux's sha256(token+name+namespace), it is an +// HMAC over "namespace/name", which no two receivers sharing a token can collide on. + +// ReceiverPath is the path a GitReceiver is served on: the prefix and the hex HMAC-SHA256 of +// "/", keyed by the receiver token. +func ReceiverPath(token []byte, namespace, name string) string { + mac := hmac.New(sha256.New, token) + mac.Write([]byte(namespace + "/" + name)) + return ReceiverPathPrefix + hex.EncodeToString(mac.Sum(nil)) +} + +// GitReceiverStatus defines the observed state of GitReceiver. +type GitReceiverStatus struct { + // ObservedGeneration is the latest generation observed by the controller. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions report whether this GitReceiver serves deliveries: the Ready summary plus the + // kstatus Reconciling/Stalled pair. Ready is True once the token Secret is readable and the + // receiver listener is on. + // +optional + // +listType=map + // +listMapKey=type + // +patchMergeKey=type + // +patchStrategy=merge + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` + + // WebhookPath is the path this GitReceiver is served on, under the receiver listener: + // /git-push/ and an HMAC of this GitReceiver's namespace and name, keyed by the token. Point the + // Git host's push webhook at the listener plus this path. Empty while the token Secret or its + // "token" key is missing. + // +optional + WebhookPath string `json:"webhookPath,omitempty"` +} + +// A delivery is routed by the repository it names, not by a reference from each GitProvider: a push +// event carries little more than a repository and a branch, and a reference bought nothing but a +// line of wiring per provider and, for a GitHub App, an admission in every namespace. Waking across +// namespaces grants nothing, because a woken worker reads the remote with its own provider's +// credential, and it holds at most one pending look however many deliveries arrive. That bounds the +// queue, not the rate: a stream of deliveries can keep a worker reading continuously, an +// availability cost accepted without a per-worker cooldown. + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` +// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` +// +kubebuilder:printcolumn:name="Reason",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].reason` +// +kubebuilder:printcolumn:name="Path",type=string,JSONPath=`.status.webhookPath`,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` + +// GitReceiver is the endpoint a Git host notifies on a push. A delivery to it authenticates against +// its token, names a repository and a branch, and wakes the branch worker of every GitProvider, in +// any namespace, whose spec.url the delivery names, for that branch. A provider reaching its host +// through an alias (another host name or port) is not matched, and finds pushes by its periodic +// refresh. A delivery is only a hint: the branch worker reads the remote with its own provider's +// credential to find out what happened, so nothing a delivery says can choose a URL, a credential +// or a commit. GitBranch.status.lastPushNoticeAt shows that deliveries reach a branch. +type GitReceiver struct { + metav1.TypeMeta `json:",inline"` + + // metadata is a standard object metadata + // +optional + metav1.ObjectMeta `json:"metadata,omitempty,omitzero"` + + // spec defines the desired state of GitReceiver + // +required + Spec GitReceiverSpec `json:"spec"` + + // status defines the observed state of GitReceiver + // +optional + Status GitReceiverStatus `json:"status,omitempty,omitzero"` +} + +// Key is this GitReceiver's namespace and name. +func (r *GitReceiver) Key() types.NamespacedName { + return types.NamespacedName{Namespace: r.Namespace, Name: r.Name} +} + +// +kubebuilder:object:root=true + +// GitReceiverList contains a list of GitReceiver. +type GitReceiverList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + + Items []GitReceiver `json:"items"` +} + +func init() { + SchemeBuilder.Register(&GitReceiver{}, &GitReceiverList{}) +} diff --git a/api/v1alpha3/gitreceiver_types_test.go b/api/v1alpha3/gitreceiver_types_test.go new file mode 100644 index 00000000..1812ef99 --- /dev/null +++ b/api/v1alpha3/gitreceiver_types_test.go @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha3 + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +func TestGitReceiver_Key(t *testing.T) { + receiver := &GitReceiver{ObjectMeta: metav1.ObjectMeta{Name: "hook", Namespace: "platform"}} + assert.Equal(t, types.NamespacedName{Namespace: "platform", Name: "hook"}, receiver.Key()) +} + +// TestReceiverPath_KeyedByTokenAndIdentity is the path's whole contract: it names neither the +// namespace nor the receiver, and changes with each of the three inputs. +func TestReceiverPath_KeyedByTokenAndIdentity(t *testing.T) { + path := ReceiverPath([]byte("token"), "platform", "hook") + assert.Regexp(t, `^/git-push/[0-9a-f]{64}$`, path) + assert.NotContains(t, path, "platform") + assert.NotContains(t, path, "hook") + assert.Equal(t, path, ReceiverPath([]byte("token"), "platform", "hook"), "the path is stable") + assert.NotEqual(t, path, ReceiverPath([]byte("other"), "platform", "hook")) + assert.NotEqual(t, path, ReceiverPath([]byte("token"), "tenant-a", "hook")) + assert.NotEqual(t, path, ReceiverPath([]byte("token"), "platform", "other")) +} diff --git a/api/v1alpha3/status_accessors.go b/api/v1alpha3/status_accessors.go index 5c28d4d6..fbf7c7cd 100644 --- a/api/v1alpha3/status_accessors.go +++ b/api/v1alpha3/status_accessors.go @@ -39,6 +39,14 @@ func (p *GitProvider) SetObservedGeneration(generation int64) { // StatusConditions returns the condition set to write through. func (p *GitProvider) StatusConditions() *[]metav1.Condition { return &p.Status.Conditions } +// SetObservedGeneration records the generation this object's status describes. +func (r *GitReceiver) SetObservedGeneration(generation int64) { + r.Status.ObservedGeneration = generation +} + +// StatusConditions returns the condition set to write through. +func (r *GitReceiver) StatusConditions() *[]metav1.Condition { return &r.Status.Conditions } + // SetObservedGeneration records the generation this object's status describes. func (p *KubeProvider) SetObservedGeneration(generation int64) { p.Status.ObservedGeneration = generation diff --git a/api/v1alpha3/status_accessors_test.go b/api/v1alpha3/status_accessors_test.go index af52f5c2..08d4abaf 100644 --- a/api/v1alpha3/status_accessors_test.go +++ b/api/v1alpha3/status_accessors_test.go @@ -27,6 +27,7 @@ func TestStatusAccessors_ReachTheLiveStatus(t *testing.T) { gitTarget := &GitTarget{} gitProvider := &GitProvider{} + gitReceiver := &GitReceiver{} kubeProvider := &KubeProvider{} watchRule := &WatchRule{} clusterWatchRule := &ClusterWatchRule{} @@ -47,6 +48,11 @@ func TestStatusAccessors_ReachTheLiveStatus(t *testing.T) { observed: func() int64 { return gitProvider.Status.ObservedGeneration }, stored: func() []metav1.Condition { return gitProvider.Status.Conditions }, }, + { + name: "GitReceiver", object: gitReceiver, + observed: func() int64 { return gitReceiver.Status.ObservedGeneration }, + stored: func() []metav1.Condition { return gitReceiver.Status.Conditions }, + }, { name: "KubeProvider", object: kubeProvider, observed: func() int64 { return kubeProvider.Status.ObservedGeneration }, diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index 365a5a55..047ffc88 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -862,6 +862,10 @@ func (in *GitBranchStatus) DeepCopyInto(out *GitBranchStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.LastPushNoticeAt != nil { + in, out := &in.LastPushNoticeAt, &out.LastPushNoticeAt + *out = (*in).DeepCopy() + } if in.Remote != nil { in, out := &in.Remote, &out.Remote *out = new(GitBranchRemoteStatus) @@ -938,22 +942,6 @@ func (in *GitProviderList) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *GitProviderReceiver) DeepCopyInto(out *GitProviderReceiver) { - *out = *in - out.SecretRef = in.SecretRef -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitProviderReceiver. -func (in *GitProviderReceiver) DeepCopy() *GitProviderReceiver { - if in == nil { - return nil - } - out := new(GitProviderReceiver) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitProviderSpec) DeepCopyInto(out *GitProviderSpec) { *out = *in @@ -982,9 +970,9 @@ func (in *GitProviderSpec) DeepCopyInto(out *GitProviderSpec) { *out = new(CommitSpec) (*in).DeepCopyInto(*out) } - if in.Receiver != nil { - in, out := &in.Receiver, &out.Receiver - *out = new(GitProviderReceiver) + if in.RemovedReceiver != nil { + in, out := &in.RemovedReceiver, &out.RemovedReceiver + *out = new(RemovedGitProviderReceiver) **out = **in } } @@ -1025,6 +1013,103 @@ func (in *GitProviderStatus) DeepCopy() *GitProviderStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitReceiver) DeepCopyInto(out *GitReceiver) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitReceiver. +func (in *GitReceiver) DeepCopy() *GitReceiver { + if in == nil { + return nil + } + out := new(GitReceiver) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *GitReceiver) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitReceiverList) DeepCopyInto(out *GitReceiverList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]GitReceiver, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitReceiverList. +func (in *GitReceiverList) DeepCopy() *GitReceiverList { + if in == nil { + return nil + } + out := new(GitReceiverList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *GitReceiverList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitReceiverSpec) DeepCopyInto(out *GitReceiverSpec) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitReceiverSpec. +func (in *GitReceiverSpec) DeepCopy() *GitReceiverSpec { + if in == nil { + return nil + } + out := new(GitReceiverSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *GitReceiverStatus) DeepCopyInto(out *GitReceiverStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitReceiverStatus. +func (in *GitReceiverStatus) DeepCopy() *GitReceiverStatus { + if in == nil { + return nil + } + out := new(GitReceiverStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitTarget) DeepCopyInto(out *GitTarget) { *out = *in @@ -1519,6 +1604,21 @@ func (in *PrunePolicy) DeepCopy() *PrunePolicy { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RemovedGitProviderReceiver) DeepCopyInto(out *RemovedGitProviderReceiver) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RemovedGitProviderReceiver. +func (in *RemovedGitProviderReceiver) DeepCopy() *RemovedGitProviderReceiver { + if in == nil { + return nil + } + out := new(RemovedGitProviderReceiver) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResourceRule) DeepCopyInto(out *ResourceRule) { *out = *in diff --git a/charts/gitops-reverser/README.md b/charts/gitops-reverser/README.md index 3b301c83..f24d9c8f 100644 --- a/charts/gitops-reverser/README.md +++ b/charts/gitops-reverser/README.md @@ -174,7 +174,7 @@ nodeSelector: | `servers.audit.timeouts.write` | Audit-server write timeout | `30s` | | `servers.audit.timeouts.idle` | Audit-server idle timeout | `60s` | | `servers.audit.tls.secretNameOverride` | Override Secret name for audit TLS cert/key | `-audit-server-cert` | -| `servers.receiver.enabled` | Serve the push receiver, over plain HTTP, for Git hosts' push webhooks to `GitProvider.status.webhookPath`, and add a `-receiver` Service; expose it with an Ingress or Gateway route of your own that terminates TLS. See [push receiver settings](../../docs/configuration.md#push-receiver-settings) | `false` | +| `servers.receiver.enabled` | Serve the push receiver, over plain HTTP, for Git hosts' push webhooks to `GitReceiver.status.webhookPath`, and add a `-receiver` Service; expose it with an Ingress or Gateway route of your own that terminates TLS. See [push receiver settings](../../docs/configuration.md#push-receiver-settings) | `false` | | `servers.receiver.bindAddress` | host:port the push receiver binds to (`--receiver-bind-address`) | `:9292` | | `servers.receiver.port` | Push receiver container port; must match the port in `bindAddress` | `9292` | | `controllerManager.additionalSensitiveResources` | Extra Secret-shaped resource types encrypted as `resource` or `group/resource` | `[]` | @@ -283,6 +283,7 @@ This chart automatically manages the following CRDs: - **`kubeproviders.configbutler.ai`** - Source-cluster connections and namespace access policy - **`gitproviders.configbutler.ai`** - Git repository connectivity and credentials - **`gitbranches.configbutler.ai`** - One branch of a repository, and which `GitProvider` writes it +- **`gitreceivers.configbutler.ai`** - Push webhook endpoints that wake the matching `GitProvider`s - **`gittargets.configbutler.ai`** - Branch/path and optional encryption configuration - **`watchrules.configbutler.ai`** - Namespaced watch rules - **`clusterwatchrules.configbutler.ai`** - Cluster-scoped watch rules @@ -308,7 +309,7 @@ kubectl apply -f https://github.com/ConfigButler/gitops-reverser/releases/downlo To manually remove CRDs after uninstallation: ```bash -kubectl delete crd gitproviders.configbutler.ai gittargets.configbutler.ai watchrules.configbutler.ai clusterwatchrules.configbutler.ai commitrequests.configbutler.ai fluxtriggers.configbutler.ai argocdtriggers.configbutler.ai kubeproviders.configbutler.ai gitbranches.configbutler.ai +kubectl delete crd gitproviders.configbutler.ai gittargets.configbutler.ai watchrules.configbutler.ai clusterwatchrules.configbutler.ai commitrequests.configbutler.ai fluxtriggers.configbutler.ai argocdtriggers.configbutler.ai kubeproviders.configbutler.ai gitbranches.configbutler.ai gitreceivers.configbutler.ai ``` Delete the triggers while the operator still runs, before this ([Uninstallation](#uninstallation)). @@ -399,7 +400,7 @@ helm uninstall gitops-reverser --namespace gitops-reverser kubectl delete namespace gitops-reverser # Delete CRDs (optional, but removes all custom resources) -kubectl delete crd gitproviders.configbutler.ai gittargets.configbutler.ai watchrules.configbutler.ai clusterwatchrules.configbutler.ai commitrequests.configbutler.ai fluxtriggers.configbutler.ai argocdtriggers.configbutler.ai kubeproviders.configbutler.ai gitbranches.configbutler.ai +kubectl delete crd gitproviders.configbutler.ai gittargets.configbutler.ai watchrules.configbutler.ai clusterwatchrules.configbutler.ai commitrequests.configbutler.ai fluxtriggers.configbutler.ai argocdtriggers.configbutler.ai kubeproviders.configbutler.ai gitbranches.configbutler.ai gitreceivers.configbutler.ai ``` ## Troubleshooting diff --git a/charts/gitops-reverser/templates/NOTES.txt b/charts/gitops-reverser/templates/NOTES.txt index feb6da38..c9175f10 100644 --- a/charts/gitops-reverser/templates/NOTES.txt +++ b/charts/gitops-reverser/templates/NOTES.txt @@ -31,6 +31,9 @@ What the chart did for you: {{- else }} - started in configured-author mode (`attribution.enabled=false`) {{- end }} +{{- if .Values.servers.receiver.enabled }} +- started the push receiver, behind the `{{ include "gitops-reverser.receiverServiceName" . }}` Service +{{- end }} {{- if .Values.quickstart.enabled }} {{- if .Values.quickstart.createNamespace }} - created the demo namespace `{{ .Values.quickstart.namespace }}` @@ -51,6 +54,13 @@ What the chart did NOT do for you: - it did NOT create your Git credentials Secret {{- end }} - it did NOT install Valkey/Redis +{{- if .Values.servers.receiver.enabled }} +- it did NOT expose the push receiver to your Git host, nor create a GitReceiver. Route + `{{ include "gitops-reverser.receiverServiceName" . }}` from an Ingress or Gateway that terminates TLS, create a + GitReceiver with the webhook token, and register `https://` plus the GitReceiver's + `status.webhookPath` as the host's push webhook. It wakes every GitProvider whose `spec.url` a + delivery names. +{{- end }} {{- if .Values.attribution.enabled }} - it did NOT write `/etc/kubernetes/...` files on your control-plane node(s) - it did NOT create your audit policy file diff --git a/charts/gitops-reverser/values.schema.json b/charts/gitops-reverser/values.schema.json index efdcc9b4..d7ad1360 100644 --- a/charts/gitops-reverser/values.schema.json +++ b/charts/gitops-reverser/values.schema.json @@ -183,7 +183,7 @@ "properties": { "enabled": { "type": "boolean", - "description": "Serve the push receiver, over plain HTTP, for Git hosts' push webhooks to GitProvider.status.webhookPath, and render a -receiver Service; expose it with an Ingress or Gateway route that terminates TLS." + "description": "Serve the push receiver, over plain HTTP, for Git hosts' push webhooks to GitReceiver.status.webhookPath, and render a -receiver Service; expose it with an Ingress or Gateway route that terminates TLS." }, "bindAddress": { "type": "string", diff --git a/charts/gitops-reverser/values.yaml b/charts/gitops-reverser/values.yaml index 4f96af57..3b76016a 100644 --- a/charts/gitops-reverser/values.yaml +++ b/charts/gitops-reverser/values.yaml @@ -244,8 +244,9 @@ servers: secretNameOverride: "" # The push receiver: the endpoint a Git host's push webhook points at, so GitOps Reverser learns in - # seconds, instead of at the next refresh, that somebody else pushed. A GitProvider opts in with - # spec.receiver and publishes its path as status.webhookPath (/git-push/). + # seconds, instead of at the next refresh, that somebody else pushed. A GitReceiver publishes its + # path as status.webhookPath (/git-push/), and a delivery to it wakes every + # GitProvider whose spec.url it names. # Off by default: a foreign push is then still found, by the next rejected push or by # git.refreshInterval. Enabled, the chart adds a containerPort and a -receiver Service # (port service.ports.receiver). The listener is plain HTTP, because a Git host cannot present the diff --git a/cmd/main.go b/cmd/main.go index 503601fb..6de44244 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -339,6 +339,15 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "GitProvider") os.Exit(1) } + if err := (&controller.GitReceiverReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ListenerEnabled: receiverEnabled(cfg), + Recorder: mgr.GetEventRecorderFor("gitreceiver"), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "GitReceiver") + os.Exit(1) + } kubeProviderReconciler := &controller.KubeProviderReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), diff --git a/cmd/main_chart_receiver_test.go b/cmd/main_chart_receiver_test.go index 95c7b841..d5935ab9 100644 --- a/cmd/main_chart_receiver_test.go +++ b/cmd/main_chart_receiver_test.go @@ -247,3 +247,15 @@ func TestChartApplierAccess_ArgoCD(t *testing.T) { assert.Len(t, helmTemplateDocs(t, "templates/applier-rbac.yaml", "applierAccess.argocd=true", "applierAccess.flux=true"), 4, "both grants together") } + +// TestChartReceiver_NotesSayWhatIsLeftToDo tells the installer, only when the listener is on, that +// a GitReceiver and an exposed route are still theirs to make: the chart renders neither. +func TestChartReceiver_NotesSayWhatIsLeftToDo(t *testing.T) { + off := renderAuditNotes(t) + assert.NotContains(t, off, "push receiver") + + on := renderAuditNotes(t, "servers.receiver.enabled=true") + assert.Contains(t, on, "started the push receiver, behind the `gitops-reverser-receiver` Service") + assert.Contains(t, on, "nor create a GitReceiver") + assert.Contains(t, on, "every GitProvider whose `spec.url` a\n delivery names") +} diff --git a/cmd/receiver.go b/cmd/receiver.go index 0b1f95db..978f4714 100644 --- a/cmd/receiver.go +++ b/cmd/receiver.go @@ -44,7 +44,7 @@ const ( func bindReceiverFlags(fs *flag.FlagSet, cfg *appConfig) { fs.StringVar(&cfg.receiverBindAddress, "receiver-bind-address", receiverOff, "Address (host:port) the push receiver listens on, over plain HTTP: a Git host's push webhook "+ - "points here plus a GitProvider's status.webhookPath (/git-push/), so a "+ + "points here plus a GitReceiver's status.webhookPath (/git-push/), so a "+ "foreign push is noticed in seconds instead of at the next refresh. Expose it through an "+ "Ingress or Gateway that terminates TLS. Default 0 serves no receiver; foreign pushes are "+ "then still found by the next rejected push or --git-refresh-interval. Served by the pod that "+ @@ -78,13 +78,13 @@ func setupPushReceiver(mgr ctrl.Manager, cfg appConfig, notifier webhookhandler. "next rejected push or the periodic refresh") return } - // A route is found through this index on the cached GitProviders, so a path nothing publishes + // A route is found through this index on the cached GitReceivers, so a path nothing publishes // costs no API read. - fatalIfErr(mgr.GetFieldIndexer().IndexField(context.Background(), &configbutleraiv1alpha3.GitProvider{}, - webhookhandler.GitProviderWebhookPathIndex, webhookhandler.IndexGitProviderWebhookPath), - "unable to index GitProviders by status.webhookPath") - // The manager client reads GitProviders from its cache and Secrets straight from the API - // server (Secrets are excluded from the cache), so a rotated token is used on the next delivery. + fatalIfErr(mgr.GetFieldIndexer().IndexField(context.Background(), &configbutleraiv1alpha3.GitReceiver{}, + webhookhandler.GitReceiverWebhookPathIndex, webhookhandler.IndexGitReceiverWebhookPath), + "unable to index GitReceivers by status.webhookPath") + // The manager client reads the kinds from its cache and Secrets straight from the API server + // (Secrets are excluded from the cache), so a rotated token is used on the next delivery. handler, err := webhookhandler.NewReceiverHandler(webhookhandler.ReceiverHandlerConfig{ Reader: mgr.GetClient(), Notifier: notifier, diff --git a/config/crd/bases/configbutler.ai_gitbranches.yaml b/config/crd/bases/configbutler.ai_gitbranches.yaml index e2d06445..735e7688 100644 --- a/config/crd/bases/configbutler.ai_gitbranches.yaml +++ b/config/crd/bases/configbutler.ai_gitbranches.yaml @@ -202,6 +202,17 @@ spec: conditions. format: int32 type: integer + lastPushNoticeAt: + description: |- + LastPushNoticeAt is when a GitReceiver last delivered a push to this branch, which made the + branch worker look at once instead of at its next refresh. It records receipt only: whether + that look reached the remote is status.remote's to say. Absent means no delivery has reached + this branch: no GitReceiver is set up, the Git host's webhook does not reach one, the delivery + names the repository by a URL other than the GitProvider's spec.url, or no branch worker writes + the branch because no GitTarget references this GitBranch. It advances at most once a minute, + and is cleared when the GitProvider is recreated for another repository. + format: date-time + type: string observedGeneration: description: ObservedGeneration is the latest generation observed by the controller. diff --git a/config/crd/bases/configbutler.ai_gitproviders.yaml b/config/crd/bases/configbutler.ai_gitproviders.yaml index e85f5e17..5dcf4d43 100644 --- a/config/crd/bases/configbutler.ai_gitproviders.yaml +++ b/config/crd/bases/configbutler.ai_gitproviders.yaml @@ -207,41 +207,10 @@ spec: type: object receiver: description: |- - Receiver lets the Git host tell GitOps Reverser that somebody pushed. Point the host's push - webhook at the receiver listener plus status.webhookPath, with the Secret's token as the - webhook secret. A delivery is only a hint: it makes GitOps Reverser read where the branch is - on the remote, and what the remote says decides whether a GitTarget's applier is asked to - apply. Without a receiver, a foreign push is still found, by the next rejected push or by - the periodic refresh. - properties: - secretRef: - description: |- - SecretRef names a Secret in this namespace whose "token" key is the webhook secret. The - token also derives status.webhookPath, so rotating it moves the path: register the new path - with the new secret. - properties: - name: - description: Name of the referent. - type: string - required: - - name - type: object - type: - description: |- - Type is the payload and signature the Git host sends. "github" checks X-Hub-Signature-256, - "gitlab" checks X-Gitlab-Token, "gitea" checks X-Gitea-Signature, and "generic-hmac" checks - X-Signature: sha256= over a {"repository", "branch", "after"} body, - for a relay of your own. - enum: - - github - - gitlab - - gitea - - generic-hmac - type: string - required: - - secretRef - - type + RemovedReceiver is the old place of a GitReceiver's type and secretRef. It is never read; + setting it is refused. type: object + x-kubernetes-preserve-unknown-fields: true secretRef: description: |- SecretRef for authentication credentials (may be nil for public repos) @@ -275,6 +244,10 @@ spec: - message: spec.url is immutable; delete and recreate the GitProvider to point at a different repository rule: self.url == oldSelf.url + - message: 'spec.receiver was replaced by a GitReceiver: create one with + this type and secretRef, and remove spec.receiver; deliveries wake + every GitProvider whose spec.url they name' + rule: '!has(self.receiver)' status: description: status defines the observed state of GitProvider properties: @@ -363,12 +336,6 @@ spec: Register this as a signing key on your git platform. Only populated when commit.signing is configured and a signing key is available. type: string - webhookPath: - description: |- - WebhookPath is the path spec.receiver is served on, under the receiver listener: - /git-push/ and a hash of this provider's name and the receiver token. Empty without - spec.receiver, and while the token Secret or its "token" key is missing. - type: string type: object required: - spec diff --git a/config/crd/bases/configbutler.ai_gitreceivers.yaml b/config/crd/bases/configbutler.ai_gitreceivers.yaml new file mode 100644 index 00000000..8d12643d --- /dev/null +++ b/config/crd/bases/configbutler.ai_gitreceivers.yaml @@ -0,0 +1,186 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.22.0 + name: gitreceivers.configbutler.ai +spec: + group: configbutler.ai + names: + kind: GitReceiver + listKind: GitReceiverList + plural: gitreceivers + singular: gitreceiver + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.type + name: Type + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].reason + name: Reason + type: string + - jsonPath: .status.webhookPath + name: Path + priority: 1 + type: string + - jsonPath: .status.conditions[?(@.type=="Ready")].message + name: Message + priority: 1 + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha3 + schema: + openAPIV3Schema: + description: |- + GitReceiver is the endpoint a Git host notifies on a push. A delivery to it authenticates against + its token, names a repository and a branch, and wakes the branch worker of every GitProvider, in + any namespace, whose spec.url the delivery names, for that branch. A provider reaching its host + through an alias (another host name or port) is not matched, and finds pushes by its periodic + refresh. A delivery is only a hint: the branch worker reads the remote with its own provider's + credential to find out what happened, so nothing a delivery says can choose a URL, a credential + or a commit. GitBranch.status.lastPushNoticeAt shows that deliveries reach a branch. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec defines the desired state of GitReceiver + properties: + secretRef: + description: |- + SecretRef names a Secret in this GitReceiver's namespace whose "token" key is the webhook + secret. The token also derives status.webhookPath, so rotating it moves the path: register the + new path with the new secret. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object + x-kubernetes-validations: + - message: spec.secretRef.name must not be empty + rule: self.name != '' + type: + description: |- + Type is the payload and signature the Git host sends. "github" checks X-Hub-Signature-256, + "gitlab" checks X-Gitlab-Token, "gitea" checks X-Gitea-Signature, and "generic-hmac" checks + X-Signature: sha256= over a {"repository", "branch", "after"} body, + for a relay of your own. + enum: + - github + - gitlab + - gitea + - generic-hmac + type: string + required: + - secretRef + - type + type: object + status: + description: status defines the observed state of GitReceiver + properties: + conditions: + description: |- + Conditions report whether this GitReceiver serves deliveries: the Ready summary plus the + kstatus Reconciling/Stalled pair. Ready is True once the token Secret is readable and the + receiver listener is on. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + observedGeneration: + description: ObservedGeneration is the latest generation observed + by the controller. + format: int64 + type: integer + webhookPath: + description: |- + WebhookPath is the path this GitReceiver is served on, under the receiver listener: + /git-push/ and an HMAC of this GitReceiver's namespace and name, keyed by the token. Point the + Git host's push webhook at the listener plus this path. Empty while the token Secret or its + "token" key is missing. + type: string + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index d049ceb9..57610578 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -7,6 +7,7 @@ resources: - bases/configbutler.ai_fluxtriggers.yaml - bases/configbutler.ai_gitbranches.yaml - bases/configbutler.ai_gitproviders.yaml + - bases/configbutler.ai_gitreceivers.yaml - bases/configbutler.ai_gittargets.yaml - bases/configbutler.ai_kubeproviders.yaml - bases/configbutler.ai_watchrules.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 15bffbd5..9d5b4669 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -47,6 +47,7 @@ rules: - configbutler.ai resources: - argocdtriggers + - gitreceivers - kubeproviders verbs: - get @@ -61,6 +62,7 @@ rules: - fluxtriggers/status - gitbranches/status - gitproviders/status + - gitreceivers/status - gittargets/status - kubeproviders/status - watchrules/status diff --git a/config/samples/README.md b/config/samples/README.md index 48c6fcaa..6a53ae7b 100644 --- a/config/samples/README.md +++ b/config/samples/README.md @@ -7,6 +7,8 @@ These samples are quick starting points for common GitOps Reverser setups. repository. - `quickstart-gittarget.yaml`: Minimal `GitTarget` using a non-root `spec.path` and SOPS encryption auto-generation. - `quickstart-watchrule.yaml`: Minimal `WatchRule` for ConfigMaps. +- `gitreceiver.yaml`: A `GitReceiver` a Git host's push webhook points at, with its token Secret in + its own namespace. It wakes every `GitProvider` whose `spec.url` a delivery names. - `kubeprovider.yaml`: A `KubeProvider` for a remote cluster, with its kubeconfig Secret in its own namespace, showing `accessFrom` and `allowCrossNamespace`. - `clusterwatchrule.yaml`: Minimal `ClusterWatchRule` for cluster-scoped resources. diff --git a/config/samples/gitreceiver.yaml b/config/samples/gitreceiver.yaml new file mode 100644 index 00000000..a98fb991 --- /dev/null +++ b/config/samples/gitreceiver.yaml @@ -0,0 +1,24 @@ +# A push receiver: the endpoint a Git host's push webhook points at, so GitOps Reverser notices a +# push by somebody else in seconds instead of at the next refresh. Its token Secret lives in this +# GitReceiver's own namespace, under the key "token": +# +# kubectl -n default create secret generic example-receiver-token --from-literal=token= +# +# Once the Secret exists, status.webhookPath carries the path (/git-push/<64 hex characters>). +# Register the receiver listener's URL plus that path as the host's push webhook, with the same +# token as its secret. The operator serves the listener only with --receiver-bind-address set (chart +# value servers.receiver.enabled=true); without it this GitReceiver reports ListenerDisabled. +# +# A delivery wakes every GitProvider, in any namespace, whose spec.url names the repository it +# reports; no GitProvider names the receiver. So a GitHub App or an organization webhook, which has +# one endpoint for many repositories, needs only one. GitBranch.status.lastPushNoticeAt shows +# that deliveries reach a branch. +apiVersion: configbutler.ai/v1alpha3 +kind: GitReceiver +metadata: + name: example-receiver + namespace: default +spec: + type: github # github | gitlab | gitea | generic-hmac + secretRef: + name: example-receiver-token diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 7ad06219..9ff09cda 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -7,6 +7,72 @@ guidance that the changelog's breaking-change entries link to. We are pre-1.0, so breaking changes bump the **minor** version (release-please is configured with `bump-minor-pre-major`) rather than the major. Read the relevant entry before upgrading across it. +## `GitReceiver` replaces `GitProvider.spec.receiver` + +**Breaking.** The push receiver is a namespaced kind of its own, `GitReceiver`, and nothing names it: +a delivery wakes every `GitProvider`, in any namespace, whose `spec.url` names the repository the +delivery reports. One `GitReceiver` therefore serves every repository its Git host delivers for, so a +GitHub App or an organization webhook, which has one endpoint for many repositories, needs one route. +`GitProvider.spec.receiver` stays in the schema only so that setting it is refused at admission, +with a message naming the replacement. `GitProvider.status.webhookPath` is gone; the path is the +`GitReceiver`'s `status.webhookPath`. + +```yaml +kind: GitReceiver +metadata: {name: platform, namespace: team-a} # the provider's name keeps the webhook's path +spec: + type: github # the old spec.receiver.type + secretRef: {name: platform-receiver} # the old spec.receiver.secretRef, in this namespace +``` + +Migrate in this order: + +1. Record what you have, before upgrading the CRDs: + + ```bash + kubectl get gitproviders -A -o jsonpath='{range .items[?(@.spec.receiver)]}{.metadata.namespace}/{.metadata.name}{"\t"}{.spec.receiver.type}{"\t"}{.spec.receiver.secretRef.name}{"\t"}{.status.webhookPath}{"\n"}{end}' + ``` + +2. Upgrade. A `GitProvider` that still carries `spec.receiver` keeps working as a provider, and its + status keeps updating, but any write that changes its `spec` is refused until that same write + removes the field. Nothing is served on its old path. +3. For each, create a `GitReceiver` in the provider's namespace with the same `type` and + `secretRef`, and **give it the provider's name**: the path is the same HMAC of the namespace and + name, keyed by the same token, so the Git host's webhook needs no change. Then remove + `spec.receiver` from the provider. +4. Check `kubectl get gitreceiver`: `Ready` is `True` once its token is readable and the listener is + on. A receiver under another name publishes a new `status.webhookPath`; register that one. After + the next push, the `GitBranch`'s `status.lastPushNoticeAt` shows the delivery reached it. + +**A delivery is routed by the repository it names.** A route used to name one provider, so the +receiver read only the branch. Now the repository URLs in the payload (`repository.clone_url`, +`ssh_url` and `html_url` from GitHub and Gitea, `project.git_http_url` and `git_ssh_url` from GitLab) +are compared with every provider's `spec.url` as a branch claim compares repositories. A provider that +reaches the host under another name, such as an SSH alias (`ssh.github.com:443`), is not matched, and +its foreign pushes are found by the next rejected push or the periodic refresh instead. + +**Any namespace's `GitReceiver` can wake any matching provider.** Waking only makes a branch worker +read the remote with its own provider's credential, so a delivery cannot choose what is read, and a +worker holds at most one pending look however many deliveries arrive. That bounds the queue, not the +rate: a token holder who keeps delivering can keep a matching worker reading its remote +continuously. That availability cost is accepted; there is no per-worker cooldown. + +**A `generic-hmac` body must name its repository.** One without `repository` is refused with `400`, +because a receiver cannot tell which provider it is for. + +**A verified delivery is answered `202` with no body.** The `{"matched": N}` body is gone: any +namespace can hold a `GitReceiver`, and a count would tell its token holder whether another namespace +tracks a repository. A relay that read it reads the metric below or `lastPushNoticeAt` instead. + +**`ListenerDisabled`.** A `GitReceiver` reports `Ready=False` with this reason while the operator runs +without `--receiver-bind-address` (chart value `servers.receiver.enabled=false`). + +**The delivery metric is per receiver.** `gitopsreverser_receiver_deliveries_total` gains the +`receiver_namespace` and `receiver_name` labels, and its `unmatched` outcome is split into +`unknown_repository` (no `GitProvider` has the repository the delivery names) and `unknown_branch` +(one has, but no branch worker writes that branch). A query that read `outcome="unmatched"` reads +both. + ## `KubeProvider` replaces `ClusterProvider`; every reference names one with `spec.kubeProviderRef` **Breaking.** The source cluster is a namespaced `KubeProvider`. The cluster-scoped diff --git a/docs/api-first-publication.md b/docs/api-first-publication.md index 7ebd6c42..b5d8ae99 100644 --- a/docs/api-first-publication.md +++ b/docs/api-first-publication.md @@ -29,7 +29,7 @@ worker can plan locally and spend its network budget on publication. This guide describes the shipped behavior. Retained work is retried on the worker's own backoff; [failure timing](#failure-timing-and-memory-limits) says what that means in practice. The -[push receiver](configuration.md#receiving-push-notifications-specreceiver) lets a Git host report +[push receiver](configuration.md#gitreceiver) lets a Git host report a push, the [apply trigger design](design/apply-trigger.md) covers what follows one, and [architecture](architecture.md) covers the wider operator. @@ -338,7 +338,7 @@ idle target can hold an old view after a foreign push; `--git-refresh-interval=0 and restores the old behaviour, where the view is corrected by a later publication, resync, or explicit recheck. -The [push receiver](configuration.md#receiving-push-notifications-specreceiver) tells the worker +The [push receiver](configuration.md#gitreceiver) tells the worker that somebody pushed, in seconds rather than at the next refresh. A delivery is queued on the worker like a refresh and is only a hint: on an idle branch the worker reads the advertisement and fetches only if the branch moved; on a busy one it closes its open window and publishes, and the diff --git a/docs/architecture.md b/docs/architecture.md index 5a98a531..83902d19 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -119,9 +119,10 @@ than a side effect, and `reconcile.configbutler.ai/requestedAt` still forces a f human who wants one now. A refusal a write found is left alone: a structural read cannot see a write-boundary precondition, so only the resync that writes the affected type clears that one. -Seconds-fresh is the push receiver's job: a `GitProvider` with `spec.receiver` accepts the Git -host's push webhook. A delivery is only a hint: it makes the branch worker read the ref -advertisement, and what the remote says decides whether the branch moved. It never triggers a +Seconds-fresh is the push receiver's job: a `GitReceiver` accepts the Git host's push webhook, and +wakes the branch worker of every `GitProvider` whose `spec.url` names the repository the delivery +names, for the branch it names. A delivery is only a hint: it makes the branch worker read the ref advertisement +with its own provider's credential, and what the remote says decides whether the branch moved. It never triggers a cluster snapshot: the cluster still holds the values from before the push, so a snapshot would publish them over the incoming change. @@ -272,14 +273,16 @@ capture audit and admission with **no** resulting watch event. ## Configuration model -You configure GitOps Reverser through nine CRDs (group `configbutler.ai`, version `v1alpha3`). +You configure GitOps Reverser through ten CRDs (group `configbutler.ai`, version `v1alpha3`). `WatchRule` and `ClusterWatchRule` choose which Kubernetes resources enter the pipeline. `CommitRequest` can ask for the current window to be saved. `GitTarget` joins one source cluster to one Git destination. `KubeProvider` supplies the source connection and authorization boundary; `GitProvider` supplies the repository, credentials, commit settings, and push policy, and a -`GitBranch` holds one branch of that repository for every `GitTarget` that writes it. `FluxTrigger` -and `ArgoCDTrigger` hold the Flux `Kustomization` or Argo CD `Application` that applies a folder back, -so GitOps Reverser decides when it applies. +`GitBranch` holds one branch of that repository for every `GitTarget` that writes it. A +`GitReceiver` is the endpoint the Git host's push webhook calls; nothing references it, because a +delivery is routed by the repository it names. `FluxTrigger` and `ArgoCDTrigger` hold the Flux +`Kustomization` or Argo CD `Application` that applies a folder back, so GitOps Reverser decides +when it applies. ```mermaid graph LR @@ -291,6 +294,7 @@ graph LR GT -->|kubeProviderRef| CP[KubeProvider] GT -->|gitBranchRef| GB[GitBranch] GB -->|gitProviderRef| GP[GitProvider] + GR[GitReceiver] -.->|"wakes by spec.url"| GP KCFG[("Remote kubeconfig Secret\nprovider's namespace")] WM["Watch manager\nper-provider source context"] end @@ -306,7 +310,7 @@ graph LR CP -->|"source identity + config"| WM WM -->|"source client"| API TR -. "kustomizationRef / applicationRef" .-> APPLIER - HOST -. "push webhook to spec.receiver" .-> GP + HOST -. "push webhook to status.webhookPath" .-> GR style GP fill:#e8f4fd,stroke:#2196f3 style CP fill:#e8f4fd,stroke:#2196f3 @@ -324,7 +328,7 @@ open their source watches through that context. The contexts are keyed by the pr `namespace/name` and are released when no `GitTarget` references them, so a provider deleted and recreated under the same name is a new connection. -The applier is not one of the nine kinds. It belongs to Flux or Argo CD, lives in the cluster the +The applier is not one of the ten kinds. It belongs to Flux or Argo CD, lives in the cluster the trigger's `spec.kubeProviderRef` names (the source cluster, or a management cluster), and is reached through that `KubeProvider`'s identity. The trigger is ours: it holds the applier, and records the apply operation it started. @@ -338,6 +342,7 @@ records the apply operation it started. | `KubeProvider` | namespaced | one source-cluster connection plus namespace access policy | | `GitBranch` | namespaced | one repository branch, held for every `GitTarget` that writes it | | `GitProvider` | namespaced | a Git repo + credentials + commit/signing config | +| `GitReceiver` | namespaced | the endpoint a Git host notifies on a push, for every `GitProvider` whose `spec.url` a delivery names | | `FluxTrigger` | namespaced | the Flux `Kustomization` GitOps Reverser asks to apply, and the apply in flight | | `ArgoCDTrigger` | namespaced | the Argo CD `Application` GitOps Reverser asks to sync, and the sync in flight | @@ -496,11 +501,7 @@ Represents a Git repository and the credentials/configuration used to write it. - `spec.allowedBranches`: glob patterns that gate writable branches. - `spec.commit.committer`: committer identity (defaults to `GitOps Reverser` / `noreply@configbutler.ai`). - `spec.commit.signing`: SSH signing key reference and optional key generation. -- `spec.receiver`: optional push receiver, `type` (`github`, `gitlab`, `gitea`, `generic-hmac`) and - the `secretRef` holding its token, named after Flux's `Receiver`. - `status.signingPublicKey`: populated when signing is configured and key material is available. -- `status.webhookPath`: the receiver's path, `/git-push/`, - when `spec.receiver` is set and its token Secret holds a token. The controller verifies repository reachability and manages the signing key lifecycle. It generates an ed25519 keypair when `signing.generateWhenMissing` is set. The portable artifact across GitOps @@ -524,6 +525,8 @@ it. Every field is immutable: creating the write branch (`ParentBranchNotFound`). It does not need to be in `allowedBranches` because it is only read. - `status.repository`: the normalized repository URL, half of the key one `GitBranch` holds. +- `status.lastPushNoticeAt`: when a `GitReceiver` last delivered a push to it. It records receipt, + so it is published even when the look the delivery caused could not read the remote. - `status.remote`: where the branch is on the remote, and when that was last proved. - `status.gitTargetCount`: how many `GitTarget`s reference it, from any namespace. @@ -531,6 +534,33 @@ The controller proves the provider's credential by opening a receive-pack sessio steady interval while it succeeds), keeps the branch claim, and publishes the worker's observation. See [Git write architecture](#git-write-architecture) for the branch claim. +### GitReceiver + +- **Source**: [api/v1alpha3/gitreceiver_types.go](../api/v1alpha3/gitreceiver_types.go) +- **Controller**: [internal/controller/gitreceiver_controller.go](../internal/controller/gitreceiver_controller.go) +- **Handler**: [internal/webhook/receiver.go](../internal/webhook/receiver.go) + +The endpoint a Git host notifies on a push. No `GitProvider` names it: a delivery is routed by the +repository it names, to providers in any namespace. + +- `spec.type`: `github`, `gitlab`, `gitea` or `generic-hmac`, named after Flux's `Receiver`. +- `spec.secretRef`: a Secret in the receiver's own namespace whose `token` key is the webhook secret. +- `status.webhookPath`: `/git-push/`, once the token + Secret holds a token. `Ready` is `False` with `ListenerDisabled` while the operator serves no + listener. + +A delivery is authenticated against the token before its body is read. The handler then reads the +repository URLs and the branch the payload names, compares the URLs with every `GitProvider`'s +`spec.url` (normalized by `internal/repourl`), and wakes the branch worker writing that branch for +each one that matches, once. The worker reads the remote with its own credential, so nothing in the +payload chooses a URL, a credential or a commit, and it holds at most one pending look, so a run of +deliveries costs one ref advertisement at a time and never crowds writes out of the queue. It does +not bound the rate: deliveries arriving during successive reads keep a worker reading +continuously. That availability cost of letting any namespace's receiver wake any matching worker +is accepted, with no per-worker cooldown (see the [security model](security-model.md)). +The `202` carries no body: a count of what was woken would tell the token holder whether another +namespace tracks a repository. + ### KubeProvider - **Source**: [api/v1alpha3/kubeprovider_types.go](../api/v1alpha3/kubeprovider_types.go) @@ -1829,8 +1859,10 @@ window kept out of Git (`not_mirrored`) or mirrored because no window was open ( `apply_ownership_conflicts_total` counts objects another applier owns that were kept out of a triggered folder; and `stale_snapshots_total{outcome}` counts resync snapshots discarded because an apply made them stale, or used after the 5-minute bound. -`receiver_deliveries_total{type,outcome}` counts receiver deliveries: `notified`, `unmatched`, -`ignored` (a ping, a tag, another event type), `rejected` (bad signature) and `invalid`. +`receiver_deliveries_total{receiver_namespace,receiver_name,type,outcome}` counts receiver +deliveries per `GitReceiver`: `notified`, `unknown_repository` (no `GitProvider` has the repository), +`unknown_branch`, `ignored` (a ping, a tag, another event type), `rejected` +(bad signature) and `invalid`. The receiver cannot tell a foreign push from an own one; the worker decides that from the advertisement, which is why a `notified` delivery of the worker's own push is followed by no request and only a foreign one by a `ForeignCommit`. See diff --git a/docs/bi-directional-argocd.md b/docs/bi-directional-argocd.md index c516459c..50f3ba15 100644 --- a/docs/bi-directional-argocd.md +++ b/docs/bi-directional-argocd.md @@ -94,13 +94,14 @@ spec: - ServerSideApply=true ``` -The GitOps Reverser side. The `GitProvider` points at the same repository (any URL form of it) and -serves a push receiver; an `ArgoCDTrigger` holds the `Application`, in the cluster the chart's -`local` `KubeProvider` names (the operator's own, through `spec.inCluster`), and the `GitTarget` -names the trigger. The `Application` is in `argocd` and the trigger in `team-a`, so the provider -lets `team-a`'s triggers reach another namespace (`allowCrossNamespace.appliers`); with the Helm -chart, set `kubeProvider.allowCrossNamespace.appliers: true` instead of declaring it. A -`KubeProvider` with `inCluster` that you declare yourself is honored only once it is listed in +The GitOps Reverser side. The `GitProvider` points at the same repository (any URL form of it), +which is how the `GitReceiver` the Git host's push webhook calls finds it; an `ArgoCDTrigger` holds +the `Application`, in the cluster the chart's `local` `KubeProvider` names (the operator's own, +through `spec.inCluster`), and the `GitTarget` names the trigger. The `Application` is in `argocd` +and the trigger in `team-a`, so the provider lets `team-a`'s triggers reach another namespace +(`allowCrossNamespace.appliers`); with the Helm chart, set +`kubeProvider.allowCrossNamespace.appliers: true` instead of declaring it. A `KubeProvider` with +`inCluster` that you declare yourself is honored only once it is listed in `--in-cluster-kube-providers`, which the chart does for its own: ```yaml @@ -125,6 +126,16 @@ stringData: token: replace-with-a-long-random-string --- apiVersion: configbutler.ai/v1alpha3 +kind: GitReceiver +metadata: + name: platform + namespace: team-a +spec: + type: github + secretRef: + name: platform-receiver +--- +apiVersion: configbutler.ai/v1alpha3 kind: GitProvider metadata: name: platform @@ -134,10 +145,6 @@ spec: allowedBranches: ["main"] secretRef: name: platform-git - receiver: - type: github - secretRef: - name: platform-receiver --- apiVersion: configbutler.ai/v1alpha3 kind: GitBranch diff --git a/docs/bi-directional-flux.md b/docs/bi-directional-flux.md index 44e8a77b..12a6983a 100644 --- a/docs/bi-directional-flux.md +++ b/docs/bi-directional-flux.md @@ -68,13 +68,14 @@ spec: interval: 1h ``` -The GitOps Reverser side. The `GitProvider` points at the same repository (any URL form of it) and -serves a push receiver; a `FluxTrigger` holds the `Kustomization`, in the cluster the chart's -`local` `KubeProvider` names (the operator's own, through `spec.inCluster`), and the `GitTarget` -names the trigger. The `Kustomization` is in `flux-system` and the trigger in `team-a`, so the -provider lets `team-a`'s triggers reach another namespace (`allowCrossNamespace.appliers`); with the -Helm chart, set `kubeProvider.allowCrossNamespace.appliers: true` instead of declaring it. A -`KubeProvider` with `inCluster` that you declare yourself is honored only once it is listed in +The GitOps Reverser side. The `GitProvider` points at the same repository (any URL form of it), +which is how the `GitReceiver` the Git host's push webhook calls finds it; a `FluxTrigger` holds the +`Kustomization`, in the cluster the chart's `local` `KubeProvider` names (the operator's own, +through `spec.inCluster`), and the `GitTarget` names the trigger. The `Kustomization` is in +`flux-system` and the trigger in `team-a`, so the provider lets `team-a`'s triggers reach another +namespace (`allowCrossNamespace.appliers`); with the Helm chart, set +`kubeProvider.allowCrossNamespace.appliers: true` instead of declaring it. A `KubeProvider` with +`inCluster` that you declare yourself is honored only once it is listed in `--in-cluster-kube-providers`, which the chart does for its own: ```yaml @@ -99,6 +100,16 @@ stringData: token: replace-with-a-long-random-string --- apiVersion: configbutler.ai/v1alpha3 +kind: GitReceiver +metadata: + name: platform + namespace: team-a +spec: + type: github + secretRef: + name: platform-receiver +--- +apiVersion: configbutler.ai/v1alpha3 kind: GitProvider metadata: name: platform @@ -108,10 +119,6 @@ spec: allowedBranches: ["main"] secretRef: name: platform-git - receiver: - type: github - secretRef: - name: platform-receiver --- apiVersion: configbutler.ai/v1alpha3 kind: GitBranch diff --git a/docs/bi-directional.md b/docs/bi-directional.md index 2b628a97..44f7ab8a 100644 --- a/docs/bi-directional.md +++ b/docs/bi-directional.md @@ -301,7 +301,7 @@ GitOps Reverser learns that the branch moved in four ways, and each feeds the sa | Signal | Latency | Configure | | --- | --- | --- | -| A push webhook from the Git host | Seconds | `GitProvider.spec.receiver` | +| A push webhook from the Git host | Seconds | A `GitReceiver`, which wakes every `GitProvider` whose `spec.url` a delivery names | | A rejected compare-and-swap push | At the next publication | Nothing: it is how every push works | | A Flux `GitRepository` that fetched another commit than the one asked for | At the next apply attempt | Nothing: the branch worker then looks at the remote, as for a push webhook | | The periodic refresh | Within `--git-refresh-interval` (`10m`) plus the 5-minute reconcile tick | Nothing: on by default | @@ -312,26 +312,33 @@ To serve the receiver: (`servers.receiver.bindAddress`). The chart adds a `-receiver` `Service`. The listener is plain HTTP; expose it to your Git host with an Ingress or Gateway route that terminates TLS, the way Flux's `webhook-receiver` is exposed. -2. Create a Secret with the webhook secret under the key `token`, in the `GitProvider`'s namespace. - It is the key a Flux `Receiver` Secret uses, so an existing one can be copied over. -3. Set `GitProvider.spec.receiver` with that Secret and the host's `type`: `github`, `gitlab`, - `gitea`, or `generic-hmac` for a relay of your own. -4. Read the path from `status.webhookPath` and register `https://` as a push - webhook with the same secret: +2. Create a Secret with the webhook secret under the key `token`. It is the key a Flux `Receiver` + Secret uses, so an existing one can be copied over. +3. Create a `GitReceiver` in the Secret's namespace, naming the Secret and the host's `type`: + `github`, `gitlab`, `gitea`, or `generic-hmac` for a relay of your own. No `GitProvider` names + it: a delivery wakes every provider, in any namespace, whose `spec.url` names the repository the + delivery reports, so a GitHub App or an organization webhook needs one `GitReceiver`. +4. Read the path from the `GitReceiver`'s `status.webhookPath` and register + `https://` as a push webhook with the same secret: ```bash - kubectl get gitprovider platform -n team-a -o jsonpath='{.status.webhookPath}' + kubectl get gitreceiver platform -n team-a -o jsonpath='{.status.webhookPath}' # /git-push/3f9a…c41e ``` The path is derived from the token, as a Flux `Receiver` path is. A wrong path and a wrong secret both answer `404`; the operator's log says which. Rotating the token moves the path. - -A delivery is only a hint. GitOps Reverser reads the branch's ref advertisement and decides from -what the remote says, so a late delivery, a duplicate, or one for its own push applies nothing it -should not. When it cannot tell whether a commit is its own, it treats it as foreign. The -[configuration reference](configuration.md#receiving-push-notifications-specreceiver) has every -field. +5. Push, and check the `GitBranch`: `status.lastPushNoticeAt` is set once a delivery reached + it, even if the look it caused could not read the remote. Still absent means the host does not + reach the receiver, the delivery names the repository by a URL other than the provider's + `spec.url` (such as an SSH alias), or no branch worker writes the branch because no `GitTarget` + references the `GitBranch`; the [delivery metric](configuration.md#gitreceiver) tells these apart. + +A delivery is only a hint. It wakes the branch worker of each `GitProvider` whose repository and +branch it names, and GitOps Reverser reads the branch's ref advertisement and +decides from what the remote says, so a late delivery, a duplicate, or one for its own push applies +nothing it should not. When it cannot tell whether a commit is its own, it treats it as foreign. The +[configuration reference](configuration.md#gitreceiver) has every field. **A push never triggers a cluster snapshot.** The cluster still holds the values from before the push, so a snapshot would publish them over the incoming change. diff --git a/docs/components.md b/docs/components.md index d67a49e0..e80182d5 100644 --- a/docs/components.md +++ b/docs/components.md @@ -76,7 +76,7 @@ The operator's own API surface and the reconcilers behind it. | Component | Path | Role | |---|---|---| -| CRD types | [`api/v1alpha3/`](../api/v1alpha3/) | the nine kinds users apply | +| CRD types | [`api/v1alpha3/`](../api/v1alpha3/) | the ten kinds users apply | | Reconcilers | [`internal/controller/`](../internal/controller/) | one per kind, plus shared condition and status helpers | | Admission handlers | [`internal/webhook/`](../internal/webhook/) | the always-allow observer and `validate-operator-types`, which captures a `CommitRequest` submitter | | Namespace admission | [`internal/authz/`](../internal/authz/) | the `KubeProvider` and `GitProvider` namespace-admission decisions (own namespace, then `spec.accessFrom`) and the `spec.allowCrossNamespace` gates, in their own package because several call sites need the same answer | @@ -125,7 +125,7 @@ write to that branch goes through its single event loop. | Manifest report | [`internal/manifestreport/`](../internal/manifestreport/) | projects a live object into a comparable manifest | | Encryption and signing | [`internal/git/sops_encryptor.go`](../internal/git/sops_encryptor.go), [`internal/sshsig/`](../internal/sshsig/) | SOPS for sensitive types, SSH signatures for commits | | Apply requests | [`internal/applier/`](../internal/applier/) | the Flux and Argo CD clients, the dispatcher that carries out each apply attempt a branch worker decides, and the trigger's `Configured` check | -| Push receiver | [`internal/webhook/receiver.go`](../internal/webhook/receiver.go) | `/git-push/` deliveries from a Git host, handed to the branch worker as a hint to read the remote | +| Push receiver | [`internal/webhook/receiver.go`](../internal/webhook/receiver.go) | `/git-push/` deliveries to a `GitReceiver`, routed by the repository and branch they name to the branch worker of each `GitProvider` whose `spec.url` matches, as a hint to read the remote | ### Attribution diff --git a/docs/configuration.md b/docs/configuration.md index ce115020..1857f883 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -39,7 +39,8 @@ The usual flow is: timers rather than the target's, and optionally with your message. 6. Only when Flux or Argo CD also applies that same folder back to the cluster: hold its `Kustomization` or `Application` with an apply trigger, name the trigger on the `GitTarget`, and - give the `GitProvider` a push receiver, so GitOps Reverser decides when Git is applied. See + point the Git host's push webhook at a [`GitReceiver`](#gitreceiver), so GitOps Reverser decides + when Git is applied. See [who applies this folder](#who-applies-this-folder-specapplytriggerref) and [bi-directional GitOps](bi-directional.md). Flux and Argo CD applying other folders need nothing. @@ -55,7 +56,7 @@ one target can be fed by multiple watch rules. ## Field overview -Ready-made manifests for all nine objects live in [`config/samples/`](../config/samples/). They +Ready-made manifests for all ten objects live in [`config/samples/`](../config/samples/). They cross-reference the same `example-provider` / `example-target` names, so the set can be applied together and edited down, rather than assembled from the fragments below. @@ -73,7 +74,6 @@ fields that need more than a line. | `knownHostsRef` | the Secret's own `known_hosts` | Optional ConfigMap or Secret sharing SSH host keys across providers. Host-key verification fails closed either way | | `commit.committer` | the operator's built-in identity | Name and email written as the commit committer. See [committer identity](#committer-identity) | | `commit.signing` | off | SSH commit signing. See [commit signing](#commit-signing) | -| `receiver` | off | Push webhook endpoint, so the Git host can say somebody pushed. See [receiving push notifications](#receiving-push-notifications-specreceiver) | ### `GitBranch` (namespaced): one branch of the repository @@ -85,6 +85,13 @@ Every field is immutable: to change one, delete the `GitBranch` and create a new | `branch` | **required** | Branch to write. Must be in the provider's `allowedBranches`. Only one `GitBranch` holds a repository branch; see [who writes a repository branch](#who-writes-a-repository-branch) | | `parentBranch` | the remote's default branch | Branch a `branch` the remote does not have yet is created from. Only read, so it need not be allowed. See [starting the write branch from another branch](#starting-the-write-branch-from-another-branch-specparentbranch) | +### `GitReceiver` (namespaced): the endpoint a Git host notifies on a push + +| Field | Default | What it does | +|---|---|---| +| `type` | **required** | Payload and signature the host sends: `github`, `gitlab`, `gitea`, or `generic-hmac` for a relay of your own. See [`GitReceiver`](#gitreceiver) | +| `secretRef` | **required** | Secret, in this namespace, whose `token` key is the webhook secret. It also derives `status.webhookPath` | + ### `KubeProvider` (namespaced): the source cluster Set exactly one of `secretRef` and `inCluster`. Both are immutable: delete and recreate to repoint. @@ -198,8 +205,9 @@ grants nothing there. ## `GitProvider` -`GitProvider` defines the Git remote, credentials, allowed branches, push receiver, and commit -behavior. +`GitProvider` defines the Git remote, credentials, allowed branches, and commit behavior. A +[`GitReceiver`](#gitreceiver) tells it about pushes by matching its `spec.url`; the provider does not +name one. The important fields are: @@ -406,64 +414,113 @@ want custom `spec.commit` behavior because the starter values do not currently e For the platform-facing behavior behind "valid signature" versus "verified badge", see [commit-signing.md](commit-signing.md). -### Receiving push notifications (`spec.receiver`) +## `GitReceiver` -`spec.receiver` lets the Git host tell GitOps Reverser that somebody else pushed. The names follow -Flux's `Receiver`, so a Flux user already knows them: +`GitReceiver` is the endpoint a Git host notifies on a push, so a push by somebody else is noticed in +seconds instead of at the next refresh. No `GitProvider` names it: a delivery wakes every +`GitProvider`, in any namespace, whose `spec.url` names the repository the delivery reports. So a +GitHub App or an organization webhook, which has one endpoint for many repositories, needs only one, +and a team that wants its own endpoint creates a `GitReceiver` in its own namespace. The names +follow Flux's `Receiver`, so a Flux user already knows them; the kind is not called `Receiver` +because Flux's sits in the same clusters. ```yaml +apiVersion: configbutler.ai/v1alpha3 +kind: GitReceiver +metadata: + name: github-app + namespace: platform spec: - url: https://github.com/acme/platform.git - receiver: - type: github # github | gitlab | gitea | generic-hmac - secretRef: - name: platform-receiver-token # key: token + type: github # github | gitlab | gitea | generic-hmac + secretRef: + name: github-app-webhook-token # in this namespace; key: token status: webhookPath: /git-push/3f9a…c41e # 64 hex characters ``` Point the host's push webhook at the receiver listener plus `status.webhookPath`, with the same -token as the webhook secret. The path is an HMAC of the provider's namespace and name, keyed by the +token as the webhook secret. The path is an HMAC of the receiver's namespace and name, keyed by the token, so it names neither and only whoever holds the token can find it. Rotating the token moves -the path: the old one stops serving at once, and the new one is published at the next reconcile -(within 5 minutes, since Secrets are not watched). `status.webhookPath` stays empty while the Secret -or its `token` key is missing. Each type verifies the request the way its host signs it: +the path: the old one stops serving at once, and the new one is published at the next reconcile. +Secrets are not watched, so while the Secret or its `token` key is missing, `status.webhookPath` is +empty and the receiver looks again every 30 seconds; once it has a token, every 5 minutes. Each type +verifies the request the way its host signs it: -| `type` | Verifies | Body | +| `type` | Verifies | Repository read from | | --- | --- | --- | -| `github` | `X-Hub-Signature-256`, an HMAC of the body | GitHub's push event | -| `gitlab` | `X-Gitlab-Token`, compared with the token | GitLab's push event | -| `gitea` | `X-Gitea-Signature`, an HMAC of the body | Gitea's push event | -| `generic-hmac` | `X-Signature: sha256=`, an HMAC of the body | `{"repository": "...", "branch": "main", "after": ""}` | - -`generic-hmac` is for a relay, such as one GitHub App webhook fanned out to many repositories. Its -header is the one Flux's `generic-hmac` `Receiver` checks. - -The signature is checked before anything in the body is read, so an unsigned request cannot probe -which repositories or branches are tracked. Every request that reaches no verified receiver gets the -same `404`, whether the path is unknown, from before a token rotation, or its signature is wrong, so -the endpoint reveals nothing about which namespaces or providers exist; the operator's log says -which it was. A verified delivery answers `202 Accepted` even when it names a branch no `GitTarget` -writes, because that is not the host's fault and an error teaches a host to disable the hook. Pushes -to other branches and tag pushes are accepted and ignored. +| `github` | `X-Hub-Signature-256`, an HMAC of the body | `repository.clone_url`, `ssh_url`, `git_url`, `html_url` | +| `gitlab` | `X-Gitlab-Token`, compared with the token | `project.git_http_url`, `git_ssh_url`, `web_url` | +| `gitea` | `X-Gitea-Signature`, an HMAC of the body | `repository.clone_url`, `ssh_url`, `html_url` | +| `generic-hmac` | `X-Signature: sha256=`, an HMAC of the body | `repository` in `{"repository": "...", "branch": "main", "after": ""}` | + +`generic-hmac` is for a relay of your own. Its header is the one Flux's `generic-hmac` `Receiver` +checks, and its body must name the repository and the branch. + +**How a delivery is routed.** The signature is checked before anything in the body is read, so an +unsigned request cannot probe which repositories or branches are tracked. A verified push names a +branch and a repository, which a host spells several ways. Every spelling is compared with the +`spec.url` of every `GitProvider`, in any namespace, the way +[a branch claim](#who-writes-a-repository-branch) compares repositories: scheme, user, a default +port, a trailing `.git` and letter case are ignored, while the host and the path are not. For each +provider that matches, the branch worker writing that branch is woken, once. A repository that +reaches the operator under another host name, such as an SSH alias, does not match. + +**Whether deliveries arrive** is on the `GitBranch`: `status.lastPushNoticeAt` is when a +`GitReceiver` last delivered a push to its branch. It records receipt, so it is set even when the +look the delivery caused could not read the remote; `status.remote` says what that look found. The +first delivery is published at once and later ones at most once a minute, and the field is cleared +when the `GitProvider` is recreated for another repository. Absent after a push has three causes, +which the receiver's `gitopsreverser_receiver_deliveries_total` outcome tells apart: + +- The host does not reach the receiver: no delivery is counted, or it is counted as `rejected`. +- The delivery names the repository by a URL other than the provider's `spec.url`, such as an SSH + alias: `unknown_repository`. +- The repository matches, but no branch worker writes that branch, because no `GitTarget` + references this `GitBranch` or it does not hold the branch: `unknown_branch`. + +Without a receiver, the provider still writes, and a foreign push is still found, by the next +rejected push or by the [periodic refresh](#keeping-an-idle-target-fresh---git-refresh-interval): +the receiver makes it seconds instead of minutes. + +A `GitReceiver` reports on `Ready` whether anything serves its path: + +| `Ready` | Reason | Meaning | +| --- | --- | --- | +| `True` | `Succeeded` | The token is readable and the listener is on | +| `False` | `SecretNotFound` | `spec.secretRef` names no Secret in the receiver's namespace | +| `False` | `SecretMalformed` | The Secret has no `token` key | +| `False` | `ListenerDisabled` | The operator serves no receiver (`--receiver-bind-address=0`). The path is still published | + +**What the host is told.** Every request that reaches no verified receiver gets the same `404`, +whether the path is unknown, from before a token rotation, or its signature is wrong, so the +endpoint reveals nothing about which namespaces or receivers exist. For a published path, the +operator's log says which it was. A verified delivery answers `202 Accepted` with no body, even when +it names a repository or a branch nothing writes, because that is not the host's fault and an error +teaches a host to disable the hook. The answer does not say what was woken: any namespace can hold a +`GitReceiver`, and a count would tell its token holder whether another namespace tracks a +repository. Pushes to +other branches, tag pushes and pings are accepted and ignored. Only a verified body that cannot be +read, or one that names no repository, answers `400`. The listener handles at most 8 deliveries at once and answers `429` above that, and a body over 2 MiB is refused with `413` before its signature is checked. A push event is a few kilobytes, and a delivery refused for either reason is still found by the periodic refresh. -A delivery is a hint, never a fact. It makes the branch worker read the branch's ref advertisement -(one request), and what the remote says decides whether the branch gained a foreign commit. A late -delivery for one of GitOps Reverser's own pushes, a duplicate, or a rollback onto one of its older -commits are all judged by the remote's answer, not by the commit the delivery names. Nothing fetches -or plans until there is something to apply, and a delivery never triggers a cluster snapshot. +A delivery is a hint, never a fact. It makes each woken branch worker read the branch's ref +advertisement (one request) with its own provider's credential, and what the remote says decides +whether the branch gained a foreign commit. The repository the payload names is only compared, never +fetched, so a delivery cannot choose a URL, a credential or a commit. A worker holds at most one +pending look: deliveries that arrive while one waits are folded into it, so a run of them costs one +ref advertisement at a time. That bounds the queue, not the rate: a steady stream of deliveries, +from any namespace's `GitReceiver`, can keep a matching worker reading its remote continuously, an +availability cost that is accepted. A late delivery for one of +GitOps Reverser's own pushes, a duplicate, or a rollback onto one of its older commits are all +judged by the remote's answer, not by the commit the delivery names. Nothing fetches or plans until +there is something to apply, and a delivery never triggers a cluster snapshot. The receiver needs its own listener, because the Git host cannot present the client certificate the audit listener requires. See [push receiver settings](#push-receiver-settings). -Without a receiver, GitOps Reverser still learns of a foreign push, from its next rejected push or -from the [periodic refresh](#keeping-an-idle-target-fresh---git-refresh-interval). The receiver -makes it seconds instead of minutes. - ## `KubeProvider` `KubeProvider` names the Kubernetes cluster a `GitTarget` mirrors **from**, and the cluster an apply @@ -989,7 +1046,7 @@ for that window. One branch worker holds one live window, bound to an author and target. Interleaved authors or targets split a burst. Atomic writes, a resync, buffer limits, shutdown, an `attach: Next` request, -a push notice from the [receiver](#receiving-push-notifications-specreceiver), and the +a push notice from the [receiver](#gitreceiver), and the [flush](bi-directional.md#flush-then-apply) before an apply can close a window early. Push cooldown is independent: a zero timer does not promise an immediate remote push. See the [complete trigger rules](spec/commit-window-refactor.md). @@ -1316,7 +1373,7 @@ target's [applier](#who-applies-this-folder-specapplytriggerref) is asked to app That makes the refresh the backstop for deployments when no push webhook reaches the operator. It is not a latency mechanism: it buys bounded staleness with no inbound surface and no Git-host configuration. Seconds-fresh is the job of the -[push receiver](#receiving-push-notifications-specreceiver). +[push receiver](#gitreceiver). ### Who applies this folder (`spec.applyTriggerRef`) @@ -2816,7 +2873,7 @@ attribution: ## Push receiver settings -The [push receiver](#receiving-push-notifications-specreceiver) is its own HTTP listener, separate +The [push receiver](#gitreceiver) is its own HTTP listener, separate from the audit listener. A Git host authenticates with an HMAC or a token over the body and cannot present the client certificate the audit listener requires, so the two cannot share one. @@ -2829,7 +2886,7 @@ present the client certificate the audit listener requires, so the two cannot sh The chart creates a `-receiver` Service when the listener is on. Expose it to your Git host with an Ingress or a Gateway route of your own that terminates TLS, the way Flux's `webhook-receiver` Service is exposed. Every request still has to carry a valid signature for the -route it names; a route whose `GitProvider` has no `spec.receiver` does not serve. +route it names, which is a `GitReceiver`'s `status.webhookPath`; any other path does not serve. The listener runs in the pod that runs the branch workers. The chart runs exactly one replica (it refuses `replicaCount` above `1`), so that is the only pod; running more than one needs ownership diff --git a/docs/definitions.md b/docs/definitions.md index c19e0407..ea1b2ea3 100644 --- a/docs/definitions.md +++ b/docs/definitions.md @@ -40,7 +40,7 @@ Almost every naming mistake in this project's history came from one ambiguity: t without a side, or "namespace" without a direction. Fix the side first and the rest of the vocabulary follows. -**Control cluster.** The cluster the operator runs in, and where its own nine kinds live. When a +**Control cluster.** The cluster the operator runs in, and where its own ten kinds live. When a document says "a namespace" with no qualifier, it means a control-cluster namespace. **Source cluster.** The cluster whose objects are mirrored into Git. A `KubeProvider` names one, by @@ -275,8 +275,12 @@ it and asks for a fresh one once Git is no longer ahead. A `GitTarget`'s initial first of each collection until one of it settles, are exempt, and its snapshots are held off for at most 5 minutes. -**Receiver.** `GitProvider.spec.receiver`, the endpoint a Git host notifies on a push. The word and -its `type` values are Flux's. A delivery to it is a hint to look, never a fact about the branch. +**Receiver.** A `GitReceiver`: the endpoint a Git host notifies on a push. Nothing references it: a +delivery wakes every `GitProvider` whose `spec.url` names the repository it reports. The word and +its `type` values are Flux's; the kind is `GitReceiver` because Flux's `Receiver` sits in the same +clusters, and in prose "the receiver" is fine. A delivery to it is a hint to look, never a fact +about the branch: it wakes the branch worker of each provider for the repository and branch it +names, and the worker reads the remote itself. ## Attribution diff --git a/docs/design/gitbranch-and-kubeprovider-plan.md b/docs/design/gitbranch-and-kubeprovider-plan.md index ca08df98..439f0e5a 100644 --- a/docs/design/gitbranch-and-kubeprovider-plan.md +++ b/docs/design/gitbranch-and-kubeprovider-plan.md @@ -1,6 +1,6 @@ # GitBranch, KubeProvider, and a phased API break -Status: **plan**, 2026-10-21. Phase 1 shipped in #429 and phase 2 in #430; phase 3 is not built. +Status: **plan**, 2026-10-21. Phase 1 shipped in #429, phase 2 in #430, and phase 3 after them. The current contract is in [architecture.md](../architecture.md) and [configuration.md](../configuration.md). This page records what we decided and in what order we build it. @@ -315,6 +315,21 @@ creating a `GitReceiver` in its own namespace, so there is one mechanism, not tw The name avoids Flux's `Receiver` kind, which sits in the same clusters. +Built with three corrections. **There is no reference and no `accessFrom`:** a delivery wakes every +`GitProvider`, in any namespace, whose `spec.url` names its repository. Once the receiver routes by +the payload's repository, a per-provider reference only narrowed a set the URL already narrows, and +cost a line of wiring per provider plus an admission in every namespace for a GitHub App. Waking +across namespaces grants nothing, because a woken worker reads the remote with its own credential; +it holds at most one pending look, and the `202` carries no count, so it says nothing about other +namespaces. The one cost left is availability: a token holder who keeps delivering can keep a +matching worker reading continuously. That is accepted for now; a per-worker cooldown with a +deferred follow-up would bound it, without bringing the reference back. Whether deliveries reach a +branch is `GitBranch.status.lastPushNoticeAt`, a fact rather than a condition about a reference. A +receiver whose token Secret is missing looks again every 30 seconds, because Secrets are not watched +and no delivery reaches it until its path is published. And a `generic-hmac` body must name its +repository, which it used to be allowed to omit, because a shared receiver cannot route one that +does not. + ## Not in these phases Each of these needs its own design when it is next on the list. None blocks phases 1 to 3. @@ -343,7 +358,5 @@ Phase 1: Phase 2 (settled): the `inCluster` list is `--in-cluster-kube-providers` (chart value `controllerManager.inClusterKubeProviders`), and the chart's provider is named `local`. -Phase 3: - -- Whether `GitReceiver` needs `accessFrom` at all, or whether only providers in its own namespace - may reference it. +Phase 3 (settled): routing is by URL alone, so a `GitReceiver` has no `accessFrom` and a +`GitProvider` no `gitReceiverRef` or `GitReceiverReady` condition (see the corrections above). diff --git a/docs/interpreting-metrics.md b/docs/interpreting-metrics.md index 5513ec47..8d145124 100644 --- a/docs/interpreting-metrics.md +++ b/docs/interpreting-metrics.md @@ -82,7 +82,7 @@ without a kubeconfig. | `resource_condition` | gauge | `kind`, `resource_namespace`, `resource_name`, `type`, `status`, `reason` | - `kind` is `GitTarget`, `WatchRule`, `ClusterWatchRule`, `GitProvider`, `GitBranch`, - `KubeProvider`, `FluxTrigger` or `ArgoCDTrigger`: the objects a human declares. + `GitReceiver`, `KubeProvider`, `FluxTrigger` or `ArgoCDTrigger`: the objects a human declares. - `type` is the kstatus trio: `Ready` (the summary), `Reconciling` (working on it) and `Stalled` (given up, and nothing will retry without a human). - `status` is `True`, `False` or `Unknown`. @@ -842,7 +842,7 @@ else pushed. See [who applies this folder](configuration.md#who-applies-this-fol | `apply_ownership_conflicts_total` | counter | `provider_namespace`, `provider_name`, `branch` | One per object another applier owns that the branch worker kept out of a folder an apply trigger applies: a live write not mirrored, or a snapshot object neither upserted nor swept. The owner is read from Flux's owner labels (`kustomize.toolkit.fluxcd.io/name` and `/namespace`) or Argo CD's tracking annotation (`argocd.argoproj.io/tracking-id`). Any non-zero rate is a misconfiguration: another `Kustomization` or `Application` applies objects into a namespace the folder mirrors, and two appliers applying one object fight each other. The worker logs each object once, naming both appliers; narrow the `WatchRule`, or move the object to one applier | | `stale_snapshots_total` | counter | `provider_namespace`, `provider_name`, `branch`, `outcome` | One per resync snapshot the branch worker found stale: collected while Git was ahead of the cluster (a commit pushed and not yet applied), collected across an apply, or arriving while Git is ahead. `outcome` is `discarded` (not decided; a fresh snapshot of its `GitTarget` is asked for once Git is no longer ahead, and the `GitTarget`'s live writes are held back by the render-fidelity gate until it lands) or `used` (the `GitTarget`'s snapshots had been held for 5 minutes while an apply kept failing, so this one was decided anyway, and whatever the unapplied commit changed in its objects is reverted, as the API-wins rule says). `discarded` comes in short bursts around applies and needs no action; a discard is not a resync failure and is not counted in `git_resync_failures_total`. Any `used` means an applier is failing for minutes at a time: read the trigger's `status.operation` and the `ApplyFailed` Events. See [stale snapshots](spec/reconcile-via-watchlist-mark-and-sweep.md#stale-snapshots-are-discarded) | | `apply_operations_unresolved` | gauge | `provider_namespace`, `provider_name`, `branch` | Apply operations on the branch that have not completed 5 minutes after they were requested. Each is still followed, holds its applier's apply window open, and holds back the applier's next request until it completes, so a value that stays above zero is an applier stuck mid-apply: read its status, and the `ApplyUnresolved` Event on the `GitTarget`s. Read at scrape time | -| `receiver_deliveries_total` | counter | `type`, `outcome` | One per verified-or-refused delivery to `/git-push/`. `type` is the `GitProvider.spec.receiver.type`; `outcome` is `notified` (handed to the branch worker), `unmatched` (no worker for that branch), `ignored` (a ping, an event other than a push, a tag or other non-branch ref, or a `generic-hmac` body naming another repository), `rejected` (a missing or wrong signature or token, or an oversized body) or `invalid` (verified but unparseable) | +| `receiver_deliveries_total` | counter | `receiver_namespace`, `receiver_name`, `type`, `outcome` | One per verified-or-refused delivery to a `GitReceiver`'s `/git-push/` path. `type` is its `spec.type`; `outcome` is `notified` (handed to at least one branch worker), `unknown_repository` (no `GitProvider`, in any namespace, has the repository the delivery names under any URL it carries), `unknown_branch` (one has, but no branch worker writes that branch for it), `ignored` (a ping, an event other than a push, or a tag or other non-branch ref), `rejected` (a missing or wrong signature or token, or an oversized body) or `invalid` (verified but unparseable, or naming no repository). Every outcome but `rejected` and `invalid` answers `202`. `unknown_repository` and `unknown_branch` are expected on an organization or GitHub App hook, which delivers for every repository and branch. On a repository's own hook, a steady `unknown_repository` rate means the payload names the repository by a URL no provider's `spec.url` matches, such as an SSH alias: the affected `GitBranch` has no `status.lastPushNoticeAt` | A delivery counted `notified` is only a hint: the worker reads the advertisement, and fetches under `git_fetches_total{reason="refresh"}` only when the branch actually moved. A `ForeignCommit` request @@ -855,7 +855,12 @@ already runs the edits it holds. # worker's backoff of up to a few minutes, until the applier is fixed. sum by (reason) (rate(gitopsreverser_apply_requests_total{outcome="failed"}[15m])) # A receiver a Git host is calling with the wrong secret. -sum by (type) (rate(gitopsreverser_receiver_deliveries_total{outcome="rejected"}[15m])) > 0 +sum by (receiver_namespace, receiver_name) ( + rate(gitopsreverser_receiver_deliveries_total{outcome="rejected"}[15m])) > 0 +# A receiver whose deliveries match no GitProvider's spec.url. Routine on an organization or GitHub +# App hook; on one repository's hook it is a URL spelled another way than the provider's. +sum by (receiver_namespace, receiver_name) ( + increase(gitopsreverser_receiver_deliveries_total{outcome="unknown_repository"}[1h])) > 0 # Edits landing while an apply runs, by branch. Withheld ones are reverted on the cluster until the # Repair that follows; ambiguous ones may have put the applier's revert in Git. sum by (provider_namespace, provider_name, branch, outcome) ( diff --git a/docs/rbac.md b/docs/rbac.md index d553ccef..e799dc93 100644 --- a/docs/rbac.md +++ b/docs/rbac.md @@ -89,8 +89,9 @@ cluster-wide `list`/`watch` of its types, and gets a clean 403 if the credential It holds **no Secret informer**: Secrets are excluded from the manager's cache ([`cmd/main.go`](../cmd/main.go), `Cache.DisableFor`), so every read is a direct `get` of a -Secret a `GitProvider`, `KubeProvider` or `GitTarget` names. Nothing in the operator calls `list` -or `watch` on Secrets. It creates and updates only the Secrets it owns (the generated signing key). +Secret a `GitProvider`, `GitReceiver`, `KubeProvider` or `GitTarget` names. Nothing in the operator +calls `list` or `watch` on Secrets. It creates and updates only the Secrets it owns (the generated +signing key). That is why the manager role asks for `get`, `create`, `update` and nothing more. @@ -110,7 +111,7 @@ Be precise about what is left, because RBAC is additive and the manager role is | Verb on `secrets` | Still granted? | Why | |---|---|---| | `list`, `watch` | **No** | The operator never enumerates Secrets. Removed from the manager role, and `selected` adds no wildcard to put them back. | -| `get` | Yes, cluster-scoped | It reads the git-creds, kubeconfig and age-key Secrets a `GitProvider`, `KubeProvider` or `GitTarget` names, each in that object's own namespace — and those objects may live in any namespace. | +| `get` | Yes, cluster-scoped | It reads the git-creds, receiver-token, kubeconfig and age-key Secrets a `GitProvider`, `GitReceiver`, `KubeProvider` or `GitTarget` names, each in that object's own namespace — and those objects may live in any namespace. | | `create`, `update` | Yes, cluster-scoped | It generates the signing key and, with `generateWhenMissing`, the age key. | So a `selected` install can still **read a Secret whose name and namespace it already @@ -118,8 +119,8 @@ knows**. It cannot discover one. That is a real reduction — enumeration is wha mirroring operator into a credential-harvesting one — but it is not zero Secret access, and this page will not pretend otherwise. -Scoping `get`/`create`/`update` down to the namespaces that hold a `GitProvider`, `KubeProvider` -or `GitTarget` needs a namespaced `Role` the chart does not yet render. Until then the escape hatch is +Scoping `get`/`create`/`update` down to the namespaces that hold a `GitProvider`, `GitReceiver`, +`KubeProvider` or `GitTarget` needs a namespaced `Role` the chart does not yet render. Until then the escape hatch is `rbac.create: false` plus your own role set. See [`future/least-privilege-remaining-work.md`](future/least-privilege-remaining-work.md). diff --git a/docs/security-model.md b/docs/security-model.md index 66249216..170ea8b1 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -34,9 +34,10 @@ To mirror live state into Git the controller must: convenient, and it means the reverser can enumerate **every Secret in the cluster**. `mode: selected` grants only the types you name. See [`rbac.md`](rbac.md). - **Read referenced Secrets.** The git-creds Secret, and the SOPS/age key Secret when encryption - is configured, both named by a `GitProvider`/`GitTarget`, and a remote cluster's kubeconfig - Secret, named by a `KubeProvider` in its own namespace. So the controller reads Secrets in every - namespace that holds one of those objects. This is a `get` by name, never a `list` or a `watch`. + is configured, both named by a `GitProvider`/`GitTarget`; a remote cluster's kubeconfig Secret, + named by a `KubeProvider` in its own namespace; and a webhook token Secret, named by a + `GitReceiver` in its own namespace. So the controller reads Secrets in every namespace that holds + one of those objects. This is a `get` by name, never a `list` or a `watch`. - **Receive audit events.** The kube-apiserver audit webhook posts events to the audit ingress. Those carry object metadata and, for some resources, request/response bodies. - **Drive an applier, only with an apply trigger.** Holding a Flux `Kustomization` takes `get` and @@ -114,7 +115,7 @@ namespace it already knows. Narrowing that further is | SOPS/age key material | Decrypts (and the public key encrypts) Secret data written to Git. | | Redis/Valkey queue | Buffers decoded audit events in transit; not an audit archive. | | Audit ingress (`/audit-webhook/`, such as `/audit-webhook/gitops-reverser.local` for the chart's provider) | Accepts audit traffic; protected by mutual TLS via cert-manager. A route is held by one namespace: a `KubeProvider` elsewhere that carries it is `Stalled` (`AuditRouteConflict`) and reads none of its facts. | -| Push receiver (`--receiver-bind-address`, off by default) | Plain HTTP for Git hosts' push webhooks. Each `GitProvider` route is a path derived from its token, and a delivery must carry a valid signature or token for it. A delivery is only a hint that makes the branch worker read the remote, never a fact about the branch. | +| Push receiver (`--receiver-bind-address`, off by default) | Plain HTTP for Git hosts' push webhooks. Each `GitReceiver` route is a path derived from its token, whose Secret lives in the receiver's own namespace, and a delivery must carry a valid signature or token for it before its body is read. A delivery is only a hint: it wakes the branch worker of every `GitProvider`, in any namespace, whose `spec.url` names the repository it reports, and that worker reads the remote with its own provider's credential. The payload's repository URL is only compared with `spec.url`, never fetched, so a delivery cannot choose a URL, a credential or a commit. A worker holds at most one pending look however many deliveries arrive, so deliveries never queue up behind its writes; but they are not rate-limited, so a token holder, in whatever namespace, who keeps delivering can keep a matching worker reading its remote continuously. That availability cost is accepted: there is no per-worker cooldown. The `202` carries no body, so it does not say whether another namespace tracks a repository. | | Generated Secret material | Signing keys and generated age keys live in cluster Secrets. | | `accessFrom` label selectors on `GitProvider` and `KubeProvider` | A selector admits every control-cluster namespace whose labels match, so it is only as strong as the right to set those labels. Whoever can label a namespace can admit it, including a tenant allowed to edit its own namespace. Select on labels only the platform can set, or list namespaces by `names`. | | `KubeProvider.spec.inCluster` and `--in-cluster-kube-providers` | `inCluster` wields the operator's own identity in the cluster it runs in. It is honored only for a `KubeProvider` the flag lists by exact `namespace/name` (no wildcards, empty by default); any other is `Stalled` (`InClusterNotAuthorized`) and never used. Anyone who can create a `KubeProvider` under a listed name wields that identity through it, so list only names in namespaces you control. | diff --git a/hack/crdfields/fields.yaml b/hack/crdfields/fields.yaml index 7a782219..0f457422 100644 --- a/hack/crdfields/fields.yaml +++ b/hack/crdfields/fields.yaml @@ -17,7 +17,7 @@ preamble: | ## Field overview - Ready-made manifests for all nine objects live in [`config/samples/`](../config/samples/). They + Ready-made manifests for all ten objects live in [`config/samples/`](../config/samples/). They cross-reference the same `example-provider` / `example-target` names, so the set can be applied together and edited down, rather than assembled from the fragments below. @@ -56,10 +56,7 @@ kinds: default: "off" doc: SSH commit signing. See [commit signing](#commit-signing) - path: receiver - default: "off" - doc: >- - Push webhook endpoint, so the Git host can say somebody pushed. See - [receiving push notifications](#receiving-push-notifications-specreceiver) + skip: replaced by `GitReceiver`, kept in the schema only so admission refuses it, naming the replacement - kind: GitBranch title: "`GitBranch` (namespaced): one branch of the repository" @@ -79,6 +76,18 @@ kinds: be allowed. See [starting the write branch from another branch](#starting-the-write-branch-from-another-branch-specparentbranch) + - kind: GitReceiver + title: "`GitReceiver` (namespaced): the endpoint a Git host notifies on a push" + fields: + - path: type + doc: >- + Payload and signature the host sends: `github`, `gitlab`, `gitea`, or `generic-hmac` for a + relay of your own. See [`GitReceiver`](#gitreceiver) + - path: secretRef + doc: >- + Secret, in this namespace, whose `token` key is the webhook secret. It also derives + `status.webhookPath` + - kind: KubeProvider title: "`KubeProvider` (namespaced): the source cluster" intro: "Set exactly one of `secretRef` and `inCluster`. Both are immutable: delete and recreate to repoint." diff --git a/internal/applier/argocd_guide_test.go b/internal/applier/argocd_guide_test.go index 980d2679..62aeac46 100644 --- a/internal/applier/argocd_guide_test.go +++ b/internal/applier/argocd_guide_test.go @@ -29,11 +29,13 @@ func TestArgoCDGuide_TheCompleteExampleIsAccepted(t *testing.T) { objects := guideExampleObjects(t, argoCDGuide) var provider v1alpha3.GitProvider + var receiver v1alpha3.GitReceiver var branch v1alpha3.GitBranch var target v1alpha3.GitTarget var trigger v1alpha3.ArgoCDTrigger var secret corev1.Secret decodeStrict(t, objects, "GitProvider", &provider) + decodeStrict(t, objects, "GitReceiver", &receiver) decodeStrict(t, objects, "GitBranch", &branch) decodeStrict(t, objects, "GitTarget", &target) decodeStrict(t, objects, v1alpha3.ApplyTriggerKindArgoCD, &trigger) @@ -51,8 +53,8 @@ func TestArgoCDGuide_TheCompleteExampleIsAccepted(t *testing.T) { target.Namespace, "the example's GitTarget names a GitBranch in its own namespace", ) - require.NotNil(t, provider.Spec.Receiver, "the example serves a push receiver") - assert.Equal(t, secret.Name, provider.Spec.Receiver.SecretRef.Name) + assert.Equal(t, secret.Name, receiver.Spec.SecretRef.Name) + assert.Equal(t, receiver.Namespace, secret.Namespace, "a GitReceiver reads its token in its own namespace") assert.NotEmpty(t, secret.StringData[v1alpha3.ReceiverTokenKey], "the receiver reads the token key") require.NotNil(t, target.Spec.ApplyTriggerRef) assert.Equal(t, v1alpha3.ApplyTriggerReference{Kind: v1alpha3.ApplyTriggerKindArgoCD, Name: trigger.Name}, diff --git a/internal/applier/flux_guide_test.go b/internal/applier/flux_guide_test.go index 04e9e334..b40d5662 100644 --- a/internal/applier/flux_guide_test.go +++ b/internal/applier/flux_guide_test.go @@ -43,11 +43,13 @@ func TestFluxGuide_TheCompleteExampleIsAccepted(t *testing.T) { objects := guideExampleObjects(t, fluxGuide) var provider v1alpha3.GitProvider + var receiver v1alpha3.GitReceiver var branch v1alpha3.GitBranch var target v1alpha3.GitTarget var trigger v1alpha3.FluxTrigger var secret corev1.Secret decodeStrict(t, objects, "GitProvider", &provider) + decodeStrict(t, objects, "GitReceiver", &receiver) decodeStrict(t, objects, "GitBranch", &branch) decodeStrict(t, objects, "GitTarget", &target) decodeStrict(t, objects, v1alpha3.ApplyTriggerKindFlux, &trigger) @@ -67,8 +69,8 @@ func TestFluxGuide_TheCompleteExampleIsAccepted(t *testing.T) { target.Namespace, "the example's GitTarget names a GitBranch in its own namespace", ) - require.NotNil(t, provider.Spec.Receiver, "the example serves a push receiver") - assert.Equal(t, secret.Name, provider.Spec.Receiver.SecretRef.Name) + assert.Equal(t, secret.Name, receiver.Spec.SecretRef.Name) + assert.Equal(t, receiver.Namespace, secret.Namespace, "a GitReceiver reads its token in its own namespace") assert.Equal(t, provider.Namespace, secret.Namespace) assert.NotEmpty(t, secret.StringData[v1alpha3.ReceiverTokenKey], "the receiver reads the token key") require.NotNil(t, target.Spec.ApplyTriggerRef) diff --git a/internal/controller/constants.go b/internal/controller/constants.go index 8ce1b699..3a6b2b46 100644 --- a/internal/controller/constants.go +++ b/internal/controller/constants.go @@ -188,6 +188,12 @@ const ( // re-checks the conflict. It is a fallback: the stream-state event of the sibling rule that // cleared it usually arrives first, but that event is best-effort. RequeueCollectionOverlapInterval = 30 * time.Second + // RequeueReceiverTokenInterval is how often a GitReceiver whose token Secret or "token" key is + // missing looks again. Secrets are not watched, so nothing else tells it the token arrived, and + // until it does no delivery can reach the receiver: Flux or Argo CD applying a GitReceiver and + // its Secret in one pass, in either order, must not leave the path unpublished for a steady + // interval. + RequeueReceiverTokenInterval = 30 * time.Second // RequeueWriteLostInterval is how soon a reconcile whose status write lost the optimistic-lock // race comes back. It is short because there is nothing to wait FOR: the write was rejected // against a resourceVersion this reconcile had already read, so the only thing that has to diff --git a/internal/controller/gitbranch_controller.go b/internal/controller/gitbranch_controller.go index 3532d462..6383ae36 100644 --- a/internal/controller/gitbranch_controller.go +++ b/internal/controller/gitbranch_controller.go @@ -175,7 +175,13 @@ func (r *GitBranchReconciler) evaluate( } key := git.BranchKeyFor(provider.Spec.URL, branch.Spec.Branch) - branch.Status.Repository = repourl.Normalize(provider.Spec.URL) + repository := repourl.Normalize(provider.Spec.URL) + if branch.Status.Repository != "" && branch.Status.Repository != repository { + // The GitProvider was recreated for another repository: a delivery for the one it left says + // nothing about this one. + branch.Status.LastPushNoticeAt = nil + } + branch.Status.Repository = repository rd := newGitBranchReadiness(key) claimant := git.BranchClaimant{ GitBranch: client.ObjectKeyFromObject(branch), @@ -230,7 +236,12 @@ func (r *GitBranchReconciler) observeWorker( ) time.Duration { repo := repoIdentityOf(provider) observed, seen := r.observeRemote(key) - heldFor := r.publishRemote(st, branch, observed, seen, repo, r.clockNow()) + now := r.clockNow() + heldFor := r.publishRemote(st, branch, observed, seen, repo, now) + if noticeHeldFor := publishPushNotice(branch, r.lastPushNotice(branch, key), now); noticeHeldFor > 0 && + (heldFor == 0 || noticeHeldFor < heldFor) { + heldFor = noticeHeldFor + } if seen && observed.ParentState == git.ParentMissing && observed.ParentRequested == branch.Spec.ParentBranch && !differentRepository(observed.Repo, repo) { @@ -249,6 +260,20 @@ func (r *GitBranchReconciler) observeWorker( return heldFor } +// lastPushNotice is when a Git host last reported a push to the repository branch key, which this +// GitBranch holds, zero when its worker has seen none. A worker still serving the GitBranch for the +// repository its GitProvider left has none to give: its notices are about that repository. +func (r *GitBranchReconciler) lastPushNotice(branch *configbutleraiv1alpha3.GitBranch, key git.BranchKey) time.Time { + if r.WorkerManager == nil { + return time.Time{} + } + worker, live := r.WorkerManager.WorkerForGitBranchAt(client.ObjectKeyFromObject(branch), key) + if !live { + return time.Time{} + } + return worker.LastPushNotice() +} + // observeClaim records the proof with the claim registry and returns the holder. func (r *GitBranchReconciler) observeClaim( key git.BranchKey, claimant git.BranchClaimant, proven bool, diff --git a/internal/controller/gitbranch_controller_test.go b/internal/controller/gitbranch_controller_test.go index da7849c3..5f0aa3d8 100644 --- a/internal/controller/gitbranch_controller_test.go +++ b/internal/controller/gitbranch_controller_test.go @@ -359,3 +359,58 @@ func TestGitBranch_ARepointedProviderMovesTheClaim(t *testing.T) { require.True(t, held) assert.Equal(t, "github.com/acme/other", key.Repository) } + +// TestGitBranch_ANoticeIsPublishedWithoutAnyLook: a delivery that reached the worker is published +// on receipt, even when nothing has proved anything about the remote, and without a status.remote +// stanza that would claim a look. +func TestGitBranch_ANoticeIsPublishedWithoutAnyLook(t *testing.T) { + f := newGitBranchFixture(t, branchProvider("platform", "acme"), + branchObject("platform", "edits", "acme", "edits", time.Hour)) + f.reconcile(t, "platform", "edits") + key := k8stypes.NamespacedName{Namespace: "platform", Name: "edits"} + require.NoError(t, f.workers.EnsureWorker(context.Background(), git.WorkerRequest{ + GitBranch: key, Provider: "acme", Branch: "edits", + Repo: git.RepoIdentity{ProviderUID: "uid-acme", URL: gitBranchTestURL}, + })) + assert.Nil(t, f.reconcile(t, "platform", "edits").Status.LastPushNoticeAt, "no delivery yet") + + require.True(t, f.workers.NotifyPush("platform", "acme", "edits", "abc")) + + got := f.reconcile(t, "platform", "edits") + require.NotNil(t, got.Status.LastPushNoticeAt) + assert.Nil(t, got.Status.Remote, "receipt is not a look at the remote") +} + +// TestGitBranch_ARepointClearsTheNotice: a delivery for the repository a recreated GitProvider left +// says nothing about the one it names now, neither as published nor from the old repository's +// worker, which still serves the GitBranch until a GitTarget reconcile replaces it. +func TestGitBranch_ARepointClearsTheNotice(t *testing.T) { + provider := branchProvider("platform", "acme") + f := newGitBranchFixture(t, provider, branchObject("platform", "main", "acme", "main", time.Hour)) + got := f.reconcile(t, "platform", "main") + got.Status.LastPushNoticeAt = &metav1.Time{Time: time.Now().Add(-time.Hour)} + require.NoError(t, f.client.Status().Update(context.Background(), got)) + require.NotNil(t, f.reconcile(t, "platform", "main").Status.LastPushNoticeAt, + "the same repository keeps it") + key := k8stypes.NamespacedName{Namespace: "platform", Name: "main"} + require.NoError(t, f.workers.EnsureWorker(context.Background(), git.WorkerRequest{ + GitBranch: key, Provider: "acme", Branch: "main", + Repo: git.RepoIdentity{ProviderUID: "uid-acme", URL: gitBranchTestURL}, + })) + require.True(t, f.workers.NotifyPush("platform", "acme", "main", "abc"), "the old repository's worker") + + stored := &configbutleraiv1alpha3.GitProvider{} + require.NoError(t, f.client.Get(context.Background(), ctrlclient.ObjectKeyFromObject(provider), stored)) + require.NoError(t, f.client.Delete(context.Background(), stored)) + recreated := branchProvider("platform", "acme") + recreated.UID = "uid-acme-2" + recreated.ResourceVersion = "" + recreated.Spec.URL = "https://github.com/acme/other.git" + require.NoError(t, f.client.Create(context.Background(), recreated)) + + got = f.reconcile(t, "platform", "main") + assert.Equal(t, "github.com/acme/other", got.Status.Repository) + _, live := f.workers.WorkerForGitBranch(key) + require.True(t, live, "precondition: the old repository's worker still serves the GitBranch") + assert.Nil(t, got.Status.LastPushNoticeAt) +} diff --git a/internal/controller/gitbranch_remote.go b/internal/controller/gitbranch_remote.go index 0739287d..2958095a 100644 --- a/internal/controller/gitbranch_remote.go +++ b/internal/controller/gitbranch_remote.go @@ -221,3 +221,27 @@ func equalParentStatus(a, b *configbutleraiv1alpha3.GitBranchParentStatus) bool } return *a == *b } + +// publishPushNotice writes status.lastPushNoticeAt from noticed, the worker's last push notice, and +// returns how long the floor holds a newer one back, zero when nothing is held. +// +// It is published on receipt, apart from status.remote: a delivery that reached the worker reached +// it whether or not the look it caused then read the remote, and the field is how a user tells a +// webhook that arrives from one that does not. A worker that has seen no notice since it started +// knows nothing newer, which is no reason to take back what was published. The floor is the remote +// stanza's interval, measured from the published notice: a busy repository is delivered to on every +// push, and a write per push would invalidate every watcher's cached copy. +func publishPushNotice(branch *configbutleraiv1alpha3.GitBranch, noticed, now time.Time) time.Duration { + published := branch.Status.LastPushNoticeAt + // metav1.Time is second-granular on the wire, so a notice is compared as it would be published. + if noticed.IsZero() || (published != nil && !noticed.Truncate(time.Second).After(published.Time)) { + return 0 + } + if published != nil { + if wait := published.Add(RemotePublicationInterval).Sub(now); wait > 0 { + return wait + } + } + branch.Status.LastPushNoticeAt = &metav1.Time{Time: noticed} + return 0 +} diff --git a/internal/controller/gitbranch_remote_test.go b/internal/controller/gitbranch_remote_test.go index 301d3933..7b9dcd96 100644 --- a/internal/controller/gitbranch_remote_test.go +++ b/internal/controller/gitbranch_remote_test.go @@ -638,3 +638,31 @@ func TestPublishRemote_ARecreatedTargetDoesNotInheritTheCooldown(t *testing.T) { "a target with nothing published has no rate to be limited to") assert.Equal(t, "bbbb", successor.Status.Remote.Commit) } + +// TestPublishPushNotice publishes the worker's latest notice, holds a newer one behind the floor, +// keeps what is published when the worker knows of none, and compares at the wire's granularity. +func TestPublishPushNotice(t *testing.T) { + start := time.Date(2026, 10, 22, 12, 0, 0, 0, time.UTC) + branch := remoteTestTarget() + + assert.Zero(t, publishPushNotice(branch, time.Time{}, start)) + assert.Nil(t, branch.Status.LastPushNoticeAt, "no notice, nothing published") + + assert.Zero(t, publishPushNotice(branch, start, start), "the first notice is published at once") + require.NotNil(t, branch.Status.LastPushNoticeAt) + assert.Equal(t, start, branch.Status.LastPushNoticeAt.Time) + + assert.Zero(t, publishPushNotice(branch, start.Add(500*time.Millisecond), start.Add(time.Second)), + "a sub-second difference the API server rounds away is not news") + assert.Equal(t, 55*time.Second, publishPushNotice(branch, start.Add(5*time.Second), start.Add(5*time.Second)), + "a newer notice waits out the floor") + assert.Equal(t, start, branch.Status.LastPushNoticeAt.Time) + + later := start.Add(RemotePublicationInterval) + assert.Zero(t, publishPushNotice(branch, start.Add(5*time.Second), later)) + assert.Equal(t, start.Add(5*time.Second), branch.Status.LastPushNoticeAt.Time) + + assert.Zero(t, publishPushNotice(branch, time.Time{}, later.Add(time.Hour))) + assert.Equal(t, start.Add(5*time.Second), branch.Status.LastPushNoticeAt.Time, + "a worker that knows of no notice keeps what was published") +} diff --git a/internal/controller/gitprovider_controller.go b/internal/controller/gitprovider_controller.go index c2aaf243..5c61c701 100644 --- a/internal/controller/gitprovider_controller.go +++ b/internal/controller/gitprovider_controller.go @@ -12,7 +12,6 @@ import ( gitclient "github.com/go-git/go-git/v6/plumbing/client" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -113,10 +112,6 @@ func (r *GitProviderReconciler) reconcileGitProvider( "GitProvider is not stalled", ) - // Ahead of every gate, and after beginStatus, which snapshots the status the patch is computed - // against: an operator wiring a Git host's webhook needs the path while the provider is still red. - r.publishWebhookPath(ctx, log, gitProvider) - if err := r.validateCommitConfiguration(gitProvider); err != nil { rd.stalled(ReasonCommitConfigInvalid, err.Error()) return r.commitProvider(ctx, st, rd) @@ -186,47 +181,6 @@ func (r *GitProviderReconciler) commitProvider( return ctrl.Result{RequeueAfter: st.requeueAfter(RequeueSteadyInterval)}, nil } -// publishWebhookPath sets status.webhookPath to the path spec.receiver is served on, and clears it -// when there is no receiver or no token to derive the path from. It says where the route is, not -// that anything serves it: the listener is a flag (--receiver-bind-address) this object cannot see. -// -// The token is read on every reconcile, so a rotated token moves the path at the next one. The -// receiver recomputes the path from the token it reads per delivery, so the old path stops serving -// at once, whatever this status still says. -func (r *GitProviderReconciler) publishWebhookPath( - ctx context.Context, - log logr.Logger, - gitProvider *configbutleraiv1alpha3.GitProvider, -) { - if gitProvider.Spec.Receiver == nil { - gitProvider.Status.WebhookPath = "" - return - } - secretName := gitProvider.Spec.Receiver.SecretRef.Name - secret, err := r.fetchSecret(ctx, secretName, gitProvider.Namespace) - if err != nil { - if !apierrors.IsNotFound(err) { - // A read that failed says nothing about the token: keep the published path. - log.Error(err, "Could not read the push receiver token Secret; status.webhookPath is kept", - "secret", secretName) - return - } - log.Info("Push receiver token Secret not found; status.webhookPath is empty until it exists", - "secret", secretName) - gitProvider.Status.WebhookPath = "" - return - } - token := secret.Data[configbutleraiv1alpha3.ReceiverTokenKey] - if len(token) == 0 { - log.Info("Push receiver token Secret has no token; status.webhookPath is empty until it does", - "secret", secretName, "key", configbutleraiv1alpha3.ReceiverTokenKey) - gitProvider.Status.WebhookPath = "" - return - } - gitProvider.Status.WebhookPath = configbutleraiv1alpha3.ReceiverPath(token, gitProvider.Namespace, - gitProvider.Name) -} - // signingKeyFailureReason classifies an ensureSigningKey failure for the Stalled reason. func signingKeyFailureReason(err error) string { switch { diff --git a/internal/controller/gitprovider_controller_test.go b/internal/controller/gitprovider_controller_test.go index 8a341af9..6a89fd58 100644 --- a/internal/controller/gitprovider_controller_test.go +++ b/internal/controller/gitprovider_controller_test.go @@ -308,51 +308,6 @@ var _ = Describe("GitProvider Controller", func() { } }) - It("publishes status.webhookPath while spec.receiver is set, and clears it when removed", func() { - // Published ahead of every gate: the remote is unreachable here, and the path is still - // what an operator needs to wire the Git host's webhook. - gitProvider = &configbutleraiv1alpha3.GitProvider{ - ObjectMeta: metav1.ObjectMeta{Name: "test-provider-receiver", Namespace: "default"}, - Spec: configbutleraiv1alpha3.GitProviderSpec{ - URL: "https://example.invalid/unreachable.git", - AllowedBranches: []string{"main"}, - Receiver: &configbutleraiv1alpha3.GitProviderReceiver{ - Type: configbutleraiv1alpha3.ReceiverTypeGitHub, - SecretRef: meta.LocalObjectReference{Name: "receiver-token"}, - }, - }, - } - token := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "receiver-token", Namespace: "default"}, - Data: map[string][]byte{configbutleraiv1alpha3.ReceiverTokenKey: []byte("receiver-test-token")}, - } - Expect(k8sClient.Create(ctx, token)).To(Succeed()) - DeferCleanup(func() { _ = k8sClient.Delete(ctx, token) }) - Expect(k8sClient.Create(ctx, gitProvider)).To(Succeed()) - - key := types.NamespacedName{Name: gitProvider.Name, Namespace: gitProvider.Namespace} - webhookPath := func(g Gomega) string { - _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: key}) - g.Expect(err).NotTo(HaveOccurred()) - published := &configbutleraiv1alpha3.GitProvider{} - g.Expect(k8sClient.Get(ctx, key, published)).To(Succeed()) - return published.Status.WebhookPath - } - - Eventually(webhookPath, "30s", "500ms").Should(Equal(configbutleraiv1alpha3.ReceiverPath( - []byte("receiver-test-token"), "default", "test-provider-receiver"))) - - Eventually(func(g Gomega) { - current := &configbutleraiv1alpha3.GitProvider{} - g.Expect(k8sClient.Get(ctx, key, current)).To(Succeed()) - current.Spec.Receiver = nil - g.Expect(k8sClient.Update(ctx, current)).To(Succeed()) - }, "10s", "200ms").Should(Succeed()) - - Eventually(webhookPath, "30s", "500ms").Should(BeEmpty(), - "a stale path would point a Git host at a route that no longer serves") - }) - It("should fail when secret is not found", func() { gitProvider = &configbutleraiv1alpha3.GitProvider{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/gitprovider_controller_unit_test.go b/internal/controller/gitprovider_controller_unit_test.go index f7d86fa3..2938dc9d 100644 --- a/internal/controller/gitprovider_controller_unit_test.go +++ b/internal/controller/gitprovider_controller_unit_test.go @@ -6,8 +6,6 @@ import ( "context" "testing" - "github.com/go-logr/logr" - meta "github.com/fluxcd/pkg/apis/meta" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -152,52 +150,3 @@ func newGitProviderTestClient(t *testing.T, objects ...runtime.Object) ctrlclien WithRuntimeObjects(objects...). Build() } - -// TestPublishWebhookPath_FollowsSpecReceiverAndToken pins status.webhookPath to spec.receiver and -// its token: derived from the token while both exist, moved when the token rotates, and cleared when -// either goes, so a stale path never points a Git host at a route that has stopped serving. -func TestPublishWebhookPath_FollowsSpecReceiverAndToken(t *testing.T) { - ctx := context.Background() - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "receiver-token", Namespace: "team-a"}, - Data: map[string][]byte{configbutleraiv1alpha3.ReceiverTokenKey: []byte("first")}, - } - c := newGitProviderTestClient(t, secret) - reconciler := &GitProviderReconciler{Client: c} - provider := &configbutleraiv1alpha3.GitProvider{ - ObjectMeta: metav1.ObjectMeta{Name: "platform-repo", Namespace: "team-a"}, - Spec: configbutleraiv1alpha3.GitProviderSpec{ - Receiver: &configbutleraiv1alpha3.GitProviderReceiver{ - Type: configbutleraiv1alpha3.ReceiverTypeGitHub, - SecretRef: meta.LocalObjectReference{Name: "receiver-token"}, - }, - }, - } - log := logr.Discard() - - reconciler.publishWebhookPath(ctx, log, provider) - first := provider.Status.WebhookPath - assert.Equal(t, configbutleraiv1alpha3.ReceiverPath([]byte("first"), "team-a", "platform-repo"), first) - assert.NotContains(t, first, "team-a", "the path must not name the namespace") - assert.NotContains(t, first, "platform-repo", "the path must not name the provider") - - secret.Data[configbutleraiv1alpha3.ReceiverTokenKey] = []byte("second") - require.NoError(t, c.Update(ctx, secret)) - reconciler.publishWebhookPath(ctx, log, provider) - assert.NotEqual(t, first, provider.Status.WebhookPath, "a rotated token moves the path") - - secret.Data = map[string][]byte{"password": []byte("second")} - require.NoError(t, c.Update(ctx, secret)) - reconciler.publishWebhookPath(ctx, log, provider) - assert.Empty(t, provider.Status.WebhookPath, "no token, no path") - - provider.Status.WebhookPath = "/git-push/stale" - require.NoError(t, c.Delete(ctx, secret)) - reconciler.publishWebhookPath(ctx, log, provider) - assert.Empty(t, provider.Status.WebhookPath, "no Secret, no path") - - provider.Status.WebhookPath = "/git-push/stale" - provider.Spec.Receiver = nil - reconciler.publishWebhookPath(ctx, log, provider) - assert.Empty(t, provider.Status.WebhookPath, "no receiver, no path") -} diff --git a/internal/controller/gitreceiver_controller.go b/internal/controller/gitreceiver_controller.go new file mode 100644 index 00000000..cd2f423a --- /dev/null +++ b/internal/controller/gitreceiver_controller.go @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + "github.com/go-logr/logr" + + configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" + "github.com/ConfigButler/gitops-reverser/internal/telemetry" +) + +// GitReceiverReasonListenerDisabled is the stall for a GitReceiver the operator does not serve: it +// runs with --receiver-bind-address=0, so nothing listens for the path it publishes. +const GitReceiverReasonListenerDisabled = "ListenerDisabled" + +// GitReceiverReconciler reconciles a GitReceiver: it reads the token Secret, publishes the path the +// receiver is served on, and reports whether anything serves it. +// +// It takes no finalizer: a delivery reads the GitReceiver and its token afresh, so nothing has to +// happen before one goes away. +type GitReceiverReconciler struct { + client.Client + + Scheme *runtime.Scheme + + // ListenerEnabled reports whether this operator serves the push receiver listener + // (--receiver-bind-address). Without it a GitReceiver publishes its path and reports + // ListenerDisabled, because no delivery can reach it. + ListenerEnabled bool + + // Recorder emits a Kubernetes Event on every persisted Ready transition; nil disables Events. + Recorder record.EventRecorder +} + +// This reconciler reads receivers and writes their status. It never creates, updates or deletes +// one, so it takes none of those verbs. Secrets are read by name (get); the token is never watched. +// +kubebuilder:rbac:groups=configbutler.ai,resources=gitreceivers,verbs=get;list;watch +// +kubebuilder:rbac:groups=configbutler.ai,resources=gitreceivers/status,verbs=get;update;patch +// +kubebuilder:rbac:groups="",resources=secrets,verbs=get +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch + +// Reconcile reads a GitReceiver's token and publishes its path and readiness. +func (r *GitReceiverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logf.FromContext(ctx).WithName("GitReceiverReconciler") + + var receiver configbutleraiv1alpha3.GitReceiver + if err := r.Get(ctx, req.NamespacedName, &receiver); err != nil { + if apierrors.IsNotFound(err) { + // The object is gone, so its condition series must stop rather than latch its last value. + telemetry.ForgetResourceConditions(conditionKindGitReceiver, req.Namespace, req.Name) + return ctrl.Result{}, nil + } + return ctrl.Result{}, err + } + if !receiver.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + st := beginStatus(r.Client, r.Recorder, &receiver) + rd, err := r.evaluate(ctx, log, &receiver) + if err != nil { + return ctrl.Result{}, err + } + st.applyReadiness(rd) + if err := st.commit(ctx); err != nil { + return ctrl.Result{}, err + } + cadence := RequeueSteadyInterval + if receiver.Status.WebhookPath == "" { + // Secrets are not watched, so a token Secret created after its GitReceiver is noticed only + // by looking again, and until it is, no delivery can reach the receiver. + cadence = RequeueReceiverTokenInterval + } + return ctrl.Result{RequeueAfter: st.requeueAfter(cadence)}, nil +} + +// evaluate publishes status.webhookPath and returns the verdict. The path is published ahead of +// every other gate: an operator wiring a Git host's webhook needs it while the receiver is still +// red, for instance before the listener is switched on. +// +// The token is read on every reconcile, so a rotated token moves the path at the next one. The +// receiver recomputes the path from the token it reads per delivery, so the old path stops serving +// at once, whatever this status still says. +func (r *GitReceiverReconciler) evaluate( + ctx context.Context, + log logr.Logger, + receiver *configbutleraiv1alpha3.GitReceiver, +) (*readiness, error) { + secretKey := client.ObjectKey{Namespace: receiver.Namespace, Name: receiver.Spec.SecretRef.Name} + var secret corev1.Secret + if err := r.Get(ctx, secretKey, &secret); err != nil { + if !apierrors.IsNotFound(err) { + // A read that failed says nothing about the token: keep the published path and retry. + return nil, fmt.Errorf("read GitReceiver token Secret %s: %w", secretKey, err) + } + log.V(1).Info("GitReceiver token Secret not found", "secret", secretKey) + receiver.Status.WebhookPath = "" + rd := newReadiness("", "GitReceiver is not stalled") + rd.stalled(ReasonSecretNotFound, fmt.Sprintf( + "spec.secretRef names Secret %s, which does not exist in the GitReceiver's namespace; "+ + "status.webhookPath is published once it does", secretKey)) + return rd, nil + } + token := secret.Data[configbutleraiv1alpha3.ReceiverTokenKey] + if len(token) == 0 { + receiver.Status.WebhookPath = "" + rd := newReadiness("", "GitReceiver is not stalled") + rd.stalled(ReasonSecretMalformed, fmt.Sprintf( + "Secret %s has no %q key; status.webhookPath is published once it does", + secretKey, configbutleraiv1alpha3.ReceiverTokenKey)) + return rd, nil + } + receiver.Status.WebhookPath = configbutleraiv1alpha3.ReceiverPath(token, receiver.Namespace, receiver.Name) + + rd := newReadiness(fmt.Sprintf("Serving %s push deliveries on %s", receiver.Spec.Type, + receiver.Status.WebhookPath), "GitReceiver is not stalled") + rd.stalledIf(!r.ListenerEnabled, GitReceiverReasonListenerDisabled, + "The operator serves no push receiver (--receiver-bind-address=0; chart value "+ + "servers.receiver.enabled=false), so no delivery reaches status.webhookPath. Foreign pushes are "+ + "still found by the next rejected push or the periodic refresh") + return rd, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *GitReceiverReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For( + &configbutleraiv1alpha3.GitReceiver{}, + builder.WithPredicates(predicate.GenerationChangedPredicate{}), + ). + Named("gitreceiver"). + Complete(r) +} diff --git a/internal/controller/gitreceiver_controller_test.go b/internal/controller/gitreceiver_controller_test.go new file mode 100644 index 00000000..7a74a1d5 --- /dev/null +++ b/internal/controller/gitreceiver_controller_test.go @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "errors" + "testing" + + meta "github.com/fluxcd/pkg/apis/meta" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + configbutleraiv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" +) + +// receiverNamespace is the namespace the GitReceiver fixtures below live in. +const receiverNamespace = "platform" + +// testReceiver is the GitReceiver platform/github-app, whose token is the Secret "receiver-token". +func testReceiver() *configbutleraiv1alpha3.GitReceiver { + return &configbutleraiv1alpha3.GitReceiver{ + ObjectMeta: metav1.ObjectMeta{Name: "github-app", Namespace: receiverNamespace, Generation: 1}, + Spec: configbutleraiv1alpha3.GitReceiverSpec{ + Type: configbutleraiv1alpha3.ReceiverTypeGitHub, + SecretRef: meta.LocalObjectReference{Name: "receiver-token"}, + }, + } +} + +// receiverTokenSecret is the Secret testReceiver reads, holding data. +func receiverTokenSecret(data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "receiver-token", Namespace: receiverNamespace}, + Data: data, + } +} + +func receiverTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, configbutleraiv1alpha3.AddToScheme(scheme)) + return scheme +} + +func newReceiverTestClient(t *testing.T, objects ...client.Object) client.Client { + t.Helper() + return fake.NewClientBuilder(). + WithScheme(receiverTestScheme(t)). + WithObjects(objects...). + WithStatusSubresource(&configbutleraiv1alpha3.GitReceiver{}). + Build() +} + +func reconcileReceiver( + t *testing.T, r *GitReceiverReconciler, key k8stypes.NamespacedName, +) (configbutleraiv1alpha3.GitReceiver, ctrl.Result) { + t.Helper() + result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key}) + require.NoError(t, err) + var got configbutleraiv1alpha3.GitReceiver + require.NoError(t, r.Get(context.Background(), key, &got)) + return got, result +} + +// TestGitReceiverReconcile_PublishesPathFromToken pins status.webhookPath to the token: derived from +// it while the Secret and its key exist, moved when the token rotates, and cleared when either goes, +// so a stale path never points a Git host at a route that has stopped serving. +func TestGitReceiverReconcile_PublishesPathFromToken(t *testing.T) { + ctx := context.Background() + receiver := testReceiver() + secret := receiverTokenSecret(map[string][]byte{configbutleraiv1alpha3.ReceiverTokenKey: []byte("first")}) + c := newReceiverTestClient(t, receiver, secret) + r := &GitReceiverReconciler{Client: c, ListenerEnabled: true} + + got, result := reconcileReceiver(t, r, receiver.Key()) + first := got.Status.WebhookPath + assert.Equal(t, configbutleraiv1alpha3.ReceiverPath([]byte("first"), receiverNamespace, "github-app"), first) + assert.NotContains(t, first, receiverNamespace, "the path must not name the namespace") + assert.NotContains(t, first, "github-app", "the path must not name the receiver") + assert.True(t, conditionIsTrue(got.Status.Conditions, ConditionTypeReady)) + assert.Equal(t, int64(1), got.Status.ObservedGeneration) + assert.Equal(t, RequeueSteadyInterval, result.RequeueAfter) + + secret.Data[configbutleraiv1alpha3.ReceiverTokenKey] = []byte("second") + require.NoError(t, c.Update(ctx, secret)) + got, _ = reconcileReceiver(t, r, receiver.Key()) + assert.NotEqual(t, first, got.Status.WebhookPath, "a rotated token moves the path") + + secret.Data = map[string][]byte{"password": []byte("second")} + require.NoError(t, c.Update(ctx, secret)) + got, result = reconcileReceiver(t, r, receiver.Key()) + assert.Empty(t, got.Status.WebhookPath, "no token, no path") + assertStalled(t, got.Status.Conditions, ReasonSecretMalformed) + assert.Equal(t, RequeueReceiverTokenInterval, result.RequeueAfter, + "nothing watches the Secret, so a receiver without a token looks again soon") + + require.NoError(t, c.Delete(ctx, secret)) + got, result = reconcileReceiver(t, r, receiver.Key()) + assert.Empty(t, got.Status.WebhookPath, "no Secret, no path") + assertStalled(t, got.Status.Conditions, ReasonSecretNotFound) + assert.Equal(t, RequeueReceiverTokenInterval, result.RequeueAfter) +} + +// TestGitReceiverReconcile_ListenerDisabled publishes the path, so the hook can be registered, and +// stalls: no delivery reaches a receiver the operator does not serve. +func TestGitReceiverReconcile_ListenerDisabled(t *testing.T) { + receiver := testReceiver() + secret := receiverTokenSecret(map[string][]byte{configbutleraiv1alpha3.ReceiverTokenKey: []byte("t")}) + r := &GitReceiverReconciler{Client: newReceiverTestClient(t, receiver, secret)} + + got, result := reconcileReceiver(t, r, receiver.Key()) + assert.NotEmpty(t, got.Status.WebhookPath, "the path is published even while nothing serves it") + assertStalled(t, got.Status.Conditions, GitReceiverReasonListenerDisabled) + assert.Equal(t, RequeueSteadyInterval, result.RequeueAfter) +} + +// TestGitReceiverReconcile_SecretReadFails keeps what is published and retries: a read that failed +// says nothing about the token. +func TestGitReceiverReconcile_SecretReadFails(t *testing.T) { + receiver := testReceiver() + receiver.Status.WebhookPath = "/git-push/kept" + c := fake.NewClientBuilder(). + WithScheme(receiverTestScheme(t)). + WithObjects(receiver). + WithStatusSubresource(&configbutleraiv1alpha3.GitReceiver{}). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, + opts ...client.GetOption) error { + if _, ok := obj.(*corev1.Secret); ok { + return errors.New("get boom") + } + return cl.Get(ctx, key, obj, opts...) + }, + }). + Build() + r := &GitReceiverReconciler{Client: c, ListenerEnabled: true} + + _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: receiver.Key()}) + require.Error(t, err) + var got configbutleraiv1alpha3.GitReceiver + require.NoError(t, c.Get(context.Background(), receiver.Key(), &got)) + assert.Equal(t, "/git-push/kept", got.Status.WebhookPath) +} + +// TestGitReceiverReconcile_Gone is not an error. +func TestGitReceiverReconcile_Gone(t *testing.T) { + r := &GitReceiverReconciler{Client: newReceiverTestClient(t)} + result, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: k8stypes.NamespacedName{Namespace: receiverNamespace, Name: "gone"}, + }) + require.NoError(t, err) + assert.Zero(t, result.RequeueAfter) +} + +func assertStalled(t *testing.T, conditions []metav1.Condition, reason string) { + t.Helper() + stalled := findCondition(conditions, ConditionTypeStalled) + require.NotNil(t, stalled) + assert.Equal(t, metav1.ConditionTrue, stalled.Status) + assert.Equal(t, reason, stalled.Reason) + assert.True(t, conditionIsFalse(conditions, ConditionTypeReady)) +} diff --git a/internal/controller/resource_condition_metrics.go b/internal/controller/resource_condition_metrics.go index e99cfa61..9923dda8 100644 --- a/internal/controller/resource_condition_metrics.go +++ b/internal/controller/resource_condition_metrics.go @@ -32,6 +32,7 @@ const ( conditionKindClusterWatchRule = "ClusterWatchRule" conditionKindGitProvider = "GitProvider" conditionKindGitBranch = "GitBranch" + conditionKindGitReceiver = "GitReceiver" conditionKindKubeProvider = "KubeProvider" conditionKindFluxTrigger = configbutleraiv1alpha3.ApplyTriggerKindFlux conditionKindArgoCDTrigger = configbutleraiv1alpha3.ApplyTriggerKindArgoCD @@ -58,6 +59,8 @@ func conditionMetricKind(obj client.Object) string { return conditionKindGitProvider case *configbutleraiv1alpha3.GitBranch: return conditionKindGitBranch + case *configbutleraiv1alpha3.GitReceiver: + return conditionKindGitReceiver case *configbutleraiv1alpha3.KubeProvider: return conditionKindKubeProvider case *configbutleraiv1alpha3.FluxTrigger: diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 392a30da..3fb4080a 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -131,6 +131,13 @@ var _ = BeforeSuite(func() { }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) + err = (&GitReceiverReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ListenerEnabled: true, + }).SetupWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + // Wired with a live route registry, as the operator is by default (--author-attribution=true): // that is what exercises the first-fact channel source and the AuditFactsReceived condition. err = (&KubeProviderReconciler{ diff --git a/internal/controller/superseded_fields_admission_test.go b/internal/controller/superseded_fields_admission_test.go index d90b1c2c..d84c4ca4 100644 --- a/internal/controller/superseded_fields_admission_test.go +++ b/internal/controller/superseded_fields_admission_test.go @@ -320,3 +320,65 @@ var _ = Describe("Retained and refused GitTarget branch fields", func() { Expect(err.Error()).To(ContainSubstring("gitBranchRef")) }) }) + +// GitProvider.spec.receiver became the GitReceiver kind. A pruned field would apply cleanly and leave +// a provider that used to be notified silently unnotified, so the old spelling stays in the schema +// and a CEL rule refuses it, naming the replacement. +var _ = Describe("Retained and refused GitProvider receiver field", func() { + legacyProvider := func(name string, spec map[string]any) *unstructured.Unstructured { + spec["url"] = "https://github.com/acme/platform.git" + spec["allowedBranches"] = []any{"main"} + return &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "configbutler.ai/v1alpha3", + "kind": "GitProvider", + "metadata": map[string]any{"name": name, "namespace": "default"}, + "spec": spec, + }} + } + + It("refuses spec.receiver in its old shape, or any other, naming the GitReceiver that replaces it", func() { + ctx := context.Background() + + for name, receiver := range map[string]any{ + "legacy-receiver": map[string]any{"type": "github", "secretRef": map[string]any{"name": "token"}}, + "empty-receiver": map[string]any{}, + } { + err := k8sClient.Create(ctx, legacyProvider(name, map[string]any{"receiver": receiver})) + Expect(err).To(HaveOccurred(), "%s: the old field must not apply cleanly", name) + Expect(err.Error()).To(ContainSubstring("spec.receiver was replaced by a GitReceiver"), name) + Expect(err.Error()).To(ContainSubstring("whose spec.url they name"), name) + } + }) + + It("requires a GitReceiver's type and secretRef, and refuses an unknown type", func() { + ctx := context.Background() + + receiver := func(name string, spec map[string]any) *unstructured.Unstructured { + return &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "configbutler.ai/v1alpha3", + "kind": "GitReceiver", + "metadata": map[string]any{"name": name, "namespace": "default"}, + "spec": spec, + }} + } + valid := receiver("valid-receiver", map[string]any{ + "type": "gitea", "secretRef": map[string]any{"name": "token"}, + }) + Expect(k8sClient.Create(ctx, valid)).To(Succeed()) + DeferCleanup(func() { _ = k8sClient.Delete(ctx, valid) }) + + for name, tc := range map[string]struct { + spec map[string]any + want string + }{ + "no-type": {map[string]any{"secretRef": map[string]any{"name": "token"}}, "type"}, + "no-secret": {map[string]any{"type": "github"}, "secretRef"}, + "empty-secret": {map[string]any{"type": "github", "secretRef": map[string]any{"name": ""}}, "secretRef.name"}, + "bitbucket": {map[string]any{"type": "bitbucket", "secretRef": map[string]any{"name": "token"}}, "type"}, + } { + err := k8sClient.Create(ctx, receiver(name, tc.spec)) + Expect(err).To(HaveOccurred(), name) + Expect(err.Error()).To(ContainSubstring(tc.want), name) + } + }) +}) diff --git a/internal/git/branch_worker.go b/internal/git/branch_worker.go index 8dd5b423..a132db3b 100644 --- a/internal/git/branch_worker.go +++ b/internal/git/branch_worker.go @@ -310,6 +310,13 @@ type BranchWorker struct { // work the channel length alone cannot see. hasUnpushedWork atomic.Bool + // pushNotices folds the push notices reported while one is already queued into it. See + // EnqueuePushNotification. + pushNotices pendingPushNotice + // lastPushNoticeAt is when a Git host last reported a push to this branch, in Unix nanoseconds, + // zero before the first. See LastPushNotice. + lastPushNoticeAt atomic.Int64 + // inflightItems counts work items accepted onto eventQueue but not yet fully // handled. It is incremented before the channel send (so it can never lag the // loop's receive) and decremented only after handleQueueItem returns. Using @@ -1386,6 +1393,12 @@ func (l *branchWorkerEventLoop) dispatch(item WorkItem) int64 { l.handleResyncRequest(req) return charge case item.Refresh != nil: + if item.Refresh.PushNotice { + // Before the remote is read, so a push reported from here on queues a look of its own. + if after, folded := l.w.pushNotices.take(); folded { + item.Refresh.After = after + } + } l.handleRefreshRequest(item.Refresh) case item.Request == nil: case item.Request.CommitMode == CommitModeAtomic: diff --git a/internal/git/git_branch_ref_test.go b/internal/git/git_branch_ref_test.go index d099c11d..8888d4cb 100644 --- a/internal/git/git_branch_ref_test.go +++ b/internal/git/git_branch_ref_test.go @@ -99,6 +99,39 @@ func TestWorkerForTarget_AndNotifyPushFindTheHoldersWorker(t *testing.T) { assert.False(t, m.NotifyPush("team-b", "acme", "main", "abc"), "nor through that GitProvider") } +// TestWorkerForGitBranchAt_OnlyTheWorkerOfThatRepositoryBranch: the GitBranch's worker is returned +// for its own key only, never for another repository's, nor another GitBranch's. +func TestWorkerForGitBranchAt_OnlyTheWorkerOfThatRepositoryBranch(t *testing.T) { + m := NewWorkerManager(nil, logr.Discard(), BranchWorkerLimits{}, types.SensitiveResourcePolicy{}) + gitBranch := testGitBranch("acme", "team-a", "main") + key := BranchKeyFor(ownerTestURL, "main") + m.workers[key] = &BranchWorker{GitBranch: gitBranch, Branch: "main"} + + worker, found := m.WorkerForGitBranchAt(gitBranch, key) + require.True(t, found) + assert.Equal(t, gitBranch, worker.GitBranch) + _, found = m.WorkerForGitBranchAt(gitBranch, BranchKeyFor("https://github.com/acme/other.git", "main")) + assert.False(t, found, "another repository's branch") + _, found = m.WorkerForGitBranchAt(testGitBranch("acme", "team-b", "main"), key) + assert.False(t, found, "another GitBranch") +} + +// TestNotifyPush_WakesTheGitBranch: a delivery reaches the GitBranch's reconcile at once, so its +// receipt is published even when the look it causes cannot read the remote. +func TestNotifyPush_WakesTheGitBranch(t *testing.T) { + m := NewWorkerManager(nil, logr.Discard(), BranchWorkerLimits{}, types.SensitiveResourcePolicy{}) + gitBranch := testGitBranch("acme", "team-a", "main") + m.workers[BranchKeyFor(ownerTestURL, "main")] = &BranchWorker{ + GitBranch: gitBranch, Branch: "main", GitProviderNamespace: "team-a", GitProviderRef: "acme", + Log: logr.Discard(), eventQueue: make(chan WorkItem, 1), + } + + require.True(t, m.NotifyPush("team-a", "acme", "main", "abc")) + + require.Len(t, m.GitBranchEvents(), 1) + assert.Equal(t, gitBranch, client.ObjectKeyFromObject((<-m.GitBranchEvents()).Object)) +} + // TestRecordRemoteObservation_WakesTheGitBranch: what the worker proves reaches the GitBranch's // reconcile at once, so status.remote follows a push within one reconcile. func TestRecordRemoteObservation_WakesTheGitBranch(t *testing.T) { diff --git a/internal/git/push_notification.go b/internal/git/push_notification.go index bb37cfac..11d31643 100644 --- a/internal/git/push_notification.go +++ b/internal/git/push_notification.go @@ -2,15 +2,26 @@ package git +import ( + "sync" + "time" +) + // EnqueuePushNotification queues a Git host's report that the branch was pushed to. // // The report is a hint and never a fact: after never decides whether the branch moved. It is queued -// as a refresh -// that reaches the remote whatever was proved recently, so the worker reads the advertisement itself -// and the observation it records decides whether the branch moved (applyInbox.observe). That one -// rule settles every awkward delivery without bookkeeping: a late delivery for an earlier push of -// this worker's finds the branch at the worker's latest push, a rollback onto an older one of its -// commits finds a tip it did not push last, and a duplicate costs one more advertisement. +// as a refresh that reaches the remote whatever was proved recently, so the worker reads the +// advertisement itself and the observation it records decides whether the branch moved +// (applyInbox.observe). That one rule settles every awkward delivery without bookkeeping: a late +// delivery for an earlier push of this worker's finds the branch at the worker's latest push, a +// rollback onto an older one of its commits finds a tip it did not push last, and a duplicate costs +// at most one more advertisement. +// +// At most one notice waits on the queue. A notice reported while one is waiting is folded into it, +// because the look the waiting one makes reads the remote after both pushes: a run of deliveries, +// from a busy repository or from whoever holds a receiver token, costs this worker one +// advertisement at a time. The waiting notice is taken off the fold when it is dispatched, before +// the remote is read, so a push reported during that read queues a look of its own. // // On a busy branch the refresh reads nothing (handleRefreshRequest): the worker closes its open // window instead and publishes, and the compare-and-swap push discovers the move. The one thing after @@ -19,6 +30,58 @@ package git // that save across two commits. That direction is safe even for a wrong after: a window left open // still closes on its timers, and its push finds any move by the compare-and-swap. func (w *BranchWorker) EnqueuePushNotification(after string) { + w.lastPushNoticeAt.Store(time.Now().UnixNano()) + if !w.pushNotices.offer(after) { + w.Log.V(1).Info("Git host reported a push; a look is already queued", "branch", w.Branch, "after", after) + return + } w.Log.V(1).Info("Git host reported a push; checking where the branch is", "branch", w.Branch, "after", after) - w.EnqueueRefresh(&RefreshRequest{PushNotice: true, After: after}) + if !w.EnqueueRefresh(&RefreshRequest{PushNotice: true, After: after}) { + // Nothing is waiting after all, so the next report must be free to queue. + w.pushNotices.take() + } +} + +// LastPushNotice is when a Git host last reported a push to this branch, zero when none has since +// this worker started. +func (w *BranchWorker) LastPushNotice() time.Time { + at := w.lastPushNoticeAt.Load() + if at == 0 { + return time.Time{} + } + return time.Unix(0, at) +} + +// pendingPushNotice is the one push notice a worker may have waiting on its queue. +type pendingPushNotice struct { + mu sync.Mutex + queued bool + // after is what the waiting notice names: the commit every folded report agreed on, or empty + // once two disagreed. Empty is the safe side: an open window treats it as a push it has not seen + // (noteBusyPushNotice), as it would have treated at least one of the reports folded into it. + after string +} + +// offer records a report naming after, and reports whether the caller has to queue a notice for it: +// false when one is already waiting, which then stands for this report too. +func (p *pendingPushNotice) offer(after string) bool { + p.mu.Lock() + defer p.mu.Unlock() + if p.queued { + if p.after != after { + p.after = "" + } + return false + } + p.queued, p.after = true, after + return true +} + +// take empties the slot and returns what the waiting notice names, and whether one was waiting. +func (p *pendingPushNotice) take() (string, bool) { + p.mu.Lock() + defer p.mu.Unlock() + after, queued := p.after, p.queued + p.queued, p.after = false, "" + return after, queued } diff --git a/internal/git/push_notification_test.go b/internal/git/push_notification_test.go new file mode 100644 index 00000000..7785fc38 --- /dev/null +++ b/internal/git/push_notification_test.go @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: Apache-2.0 + +package git + +import ( + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + configv1alpha3 "github.com/ConfigButler/gitops-reverser/api/v1alpha3" +) + +// queuedItems takes every item the worker queued for its loop. +func queuedItems(w *BranchWorker) []WorkItem { + var items []WorkItem + for { + select { + case item := <-w.eventQueue: + items = append(items, item) + default: + return items + } + } +} + +// TestPushNotice_ARunOfReportsQueuesOneLook: any number of deliveries reported while a notice waits +// cost the worker one look, and the next report after that look is dispatched queues another. What +// the folded notice names is what the open window decides on: the reports' commit when they agree, +// nothing (a push not yet seen) when they do not. +func TestPushNotice_ARunOfReportsQueuesOneLook(t *testing.T) { + for _, tc := range []struct { + name string + reports []string + wantAfter string + wantClose bool + }{ + {name: "late reports of our own push", reports: []string{"ours", "ours", "ours"}, wantAfter: "ours"}, + {name: "our push and somebody else's", reports: []string{"ours", "theirs"}, wantClose: true}, + {name: "somebody else's push, then ours", reports: []string{"theirs", "ours"}, wantClose: true}, + } { + t.Run(tc.name, func(t *testing.T) { + loop, _ := applyTriggerLoop(t, applierTestTarget("alpha", true), &configv1alpha3.GitProvider{ + ObjectMeta: metav1.ObjectMeta{Name: "test-provider", Namespace: "team-a"}, + Spec: configv1alpha3.GitProviderSpec{URL: "https://github.com/acme/platform"}, + }) + loop.w.recordRemoteObservation("ours", ObservedByPush) + loop.openWindow = &openWindow{} + + for _, after := range tc.reports { + loop.w.EnqueuePushNotification(after) + } + items := queuedItems(loop.w) + require.Len(t, items, 1, "one look stands for every report") + loop.dispatch(items[0]) + + assert.Equal(t, tc.wantAfter, items[0].Refresh.After) + assert.Equal(t, tc.wantClose, loop.pushHinted) + + loop.w.EnqueuePushNotification("later") + assert.Len(t, queuedItems(loop.w), 1, "a report after the look was dispatched queues its own") + }) + } +} + +// TestPushNotice_ADroppedLookFreesTheSlot: a notice the queue refuses is not waiting, so it must not +// fold the next report into nothing. +func TestPushNotice_ADroppedLookFreesTheSlot(t *testing.T) { + w := &BranchWorker{Log: logr.Discard(), Branch: "main", eventQueue: make(chan WorkItem)} + + w.EnqueuePushNotification("aaa") + _, waiting := w.pushNotices.take() + assert.False(t, waiting, "the refused notice left nothing waiting") + + assert.True(t, w.pushNotices.offer("bbb"), "the next report queues a look of its own") +} + +// TestPushNotice_LastPushNoticeIsWhenItWasReported records every report, folded or not, since that +// is what GitBranch.status.lastPushNoticeAt says: deliveries reach this branch. +func TestPushNotice_LastPushNoticeIsWhenItWasReported(t *testing.T) { + w := &BranchWorker{Log: logr.Discard(), Branch: "main", eventQueue: make(chan WorkItem, 1)} + assert.True(t, w.LastPushNotice().IsZero(), "no report yet") + + before := time.Now() + w.EnqueuePushNotification("aaa") + first := w.LastPushNotice() + assert.False(t, first.Before(before)) + + w.EnqueuePushNotification("bbb") + assert.False(t, w.LastPushNotice().Before(first), "a folded report is still a report") +} + +// TestDispatch_APushNoticeNotFoldedKeepsItsCommit: only a notice EnqueuePushNotification queued +// takes the fold, so a refresh queued with its own commit keeps it. +func TestDispatch_APushNoticeNotFoldedKeepsItsCommit(t *testing.T) { + loop, _ := applyTriggerLoop(t, applierTestTarget("alpha", true)) + req := &RefreshRequest{PushNotice: true, After: "ccc"} + loop.dispatch(WorkItem{Refresh: req}) + assert.Equal(t, "ccc", req.After) +} diff --git a/internal/git/refresh.go b/internal/git/refresh.go index 7ef8f15a..48c0e58f 100644 --- a/internal/git/refresh.go +++ b/internal/git/refresh.go @@ -42,18 +42,20 @@ type RefreshRequest struct { PushNotice bool // After is the commit a push notice named, or empty. It is never trusted as the branch's tip; // it only spares an open window a notice that names the tip already observed (see - // EnqueuePushNotification). + // EnqueuePushNotification). A queued notice takes it when it is dispatched, since notices that + // arrived while it waited were folded into it. After string } -// EnqueueRefresh queues a refresh, dropping it if the queue is full. +// EnqueueRefresh queues a refresh, dropping it if the queue is full or the worker is stopping, and +// reports whether it was queued. // // The drop is safe in a way it is not for a write: the next reconcile asks again, and a refresh // must never occupy the slot a write needs. It is still counted, because a refresher that has // stopped running is otherwise invisible. -func (w *BranchWorker) EnqueueRefresh(req *RefreshRequest) { +func (w *BranchWorker) EnqueueRefresh(req *RefreshRequest) bool { if req == nil { - return + return false } w.inflightItems.Add(1) // Under the same lock as every other admission, for the reason on stoppingState: a check @@ -63,15 +65,17 @@ func (w *BranchWorker) EnqueueRefresh(req *RefreshRequest) { if w.stoppingLocked() { w.releaseInflightItem() w.recordQueueRefusal(queueRefusalRefresh) - return + return false } select { case w.eventQueue <- WorkItem{Refresh: req}: + return true default: w.releaseInflightItem() w.recordQueueRefusal(queueRefusalRefresh) w.Log.V(1).Info("Event queue full, refresh dropped; the next reconcile asks again", "branch", w.Branch, "gitTarget", req.Target.String()) + return false } } diff --git a/internal/git/worker_manager.go b/internal/git/worker_manager.go index 7084ac62..0d956b3b 100644 --- a/internal/git/worker_manager.go +++ b/internal/git/worker_manager.go @@ -697,6 +697,20 @@ func (m *WorkerManager) WorkerForGitBranch(gitBranch k8stypes.NamespacedName) (* return m.workers[key], true } +// WorkerForGitBranchAt is WorkerForGitBranch, but only for the worker of key: while a GitProvider is +// recreated for another repository, the worker of the repository it left still serves the GitBranch +// until the GitTarget reconcile replaces it, and what it observed is not about key. +func (m *WorkerManager) WorkerForGitBranchAt(gitBranch k8stypes.NamespacedName, key BranchKey) (*BranchWorker, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + + worker, exists := m.workers[key] + if !exists || worker.GitBranch != gitBranch { + return nil, false + } + return worker, true +} + // WorkerForTarget is WorkerForGitBranch for the GitBranch a GitTarget references. func (m *WorkerManager) WorkerForTarget(target *configv1alpha3.GitTarget) (*BranchWorker, bool) { return m.WorkerForGitBranch(target.GitBranchKey()) @@ -899,9 +913,10 @@ func (m *WorkerManager) NeedLeaderElection() bool { return true } -// NotifyPush tells the worker for a provider's branch that the Git host reported a push to it. It -// reports whether such a worker exists. after is the commit the delivery named; it is logged and -// never trusted: the worker reads where the branch is from the remote itself. +// NotifyPush tells the worker for a provider's branch that the Git host reported a push to it, and +// wakes its GitBranch, so status.lastPushNoticeAt records the receipt even when the look it causes +// cannot read the remote. It reports whether such a worker exists. after is the commit the delivery +// named; it is logged and never trusted: the worker reads where the branch is from the remote itself. func (m *WorkerManager) NotifyPush(providerNamespace, providerName, branch, after string) bool { provider := k8stypes.NamespacedName{Namespace: providerNamespace, Name: providerName} m.mu.RLock() @@ -917,6 +932,7 @@ func (m *WorkerManager) NotifyPush(providerNamespace, providerName, branch, afte return false } found.EnqueuePushNotification(after) + m.notifyGitBranchLocked(found.GitBranch) return true } diff --git a/internal/telemetry/receiver.go b/internal/telemetry/receiver.go index feb2c914..3c227440 100644 --- a/internal/telemetry/receiver.go +++ b/internal/telemetry/receiver.go @@ -4,25 +4,30 @@ package telemetry import "go.opentelemetry.io/otel/metric" -// ReceiverDeliveriesTotal counts the deliveries the push receiver answered for a GitProvider that -// has spec.receiver, labelled by {type, outcome}. type is the receiver type (github, gitlab, -// gitea, generic-hmac). outcome is a frozen enum: +// ReceiverDeliveriesTotal counts the deliveries the push receiver answered for a GitReceiver, +// labelled by {receiver_namespace, receiver_name, type, outcome}. type is the receiver type +// (github, gitlab, gitea, generic-hmac). outcome is a frozen enum: // -// notified — verified, named a branch, and a branch worker for it was told to look. -// unmatched — verified and named a branch, but no branch worker writes it. Still a 202. -// ignored — verified and deliberately not acted on: a ping, an event other than a push, a -// tag or other non-branch ref, or a generic-hmac body naming another repository. -// rejected — refused before anything in it was read: the signature or token was absent or -// wrong, or the body was over the size cap. -// invalid — verified but not parseable, or a required field was missing (a 400). +// notified — verified, named a repository and a branch, and at least one branch +// worker for them was told to look. +// unknown_repository — verified, but no GitProvider in the cluster has the repository it names +// under any URL it carries. Still a 202, and routine on an organization or +// GitHub App hook, which delivers for every repository. +// unknown_branch — verified and for a repository a GitProvider has, but no branch worker +// writes the branch for it. Still a 202. +// ignored — verified and deliberately not acted on: a ping, an event other than a +// push, or a tag or other non-branch ref. +// rejected — refused before anything in it was read: the signature or token was absent +// or wrong, or the body was over the size cap. +// invalid — verified but not parseable, or a required field was missing (a 400). // // It cannot say whether the push was somebody else's: the receiver only hands the branch to its -// worker, and the worker decides foreign versus own by reading the remote. A delivery is a hint, +// workers, and each worker decides foreign versus own by reading the remote. A delivery is a hint, // so "notified" means "looked", never "found a foreign commit". // -// Requests that never reach a provider with a receiver (an unknown route, a wrong method, a -// missing token Secret) are not counted: there is no type to label them with, and the route and -// method are unbounded inputs from whoever can reach the listener. +// Requests that never reach a GitReceiver (an unknown route, a wrong method, a missing token +// Secret) are not counted: there is no receiver to label them with, and the route and method are +// unbounded inputs from whoever can reach the listener. // // Registered in registerCounters (exporter.go) with every other counter. var ReceiverDeliveriesTotal metric.Int64Counter diff --git a/internal/webhook/receiver.go b/internal/webhook/receiver.go index b237281e..4a49b652 100644 --- a/internal/webhook/receiver.go +++ b/internal/webhook/receiver.go @@ -38,18 +38,18 @@ const ReceiverPathPrefix = v1alpha3.ReceiverPathPrefix // receiverDigestHexLength is the length of a route's digest: a hex SHA-256. const receiverDigestHexLength = 2 * sha256.Size -// GitProviderWebhookPathIndex is the cache index the receiver finds a route's GitProvider by. It is -// registered on the manager's cache (IndexGitProviderWebhookPath), so a request for a path nothing +// GitReceiverWebhookPathIndex is the cache index the receiver finds a route's GitReceiver by. It is +// registered on the manager's cache (IndexGitReceiverWebhookPath), so a request for a path nothing // publishes costs no API read. -const GitProviderWebhookPathIndex = "status.webhookPath" +const GitReceiverWebhookPathIndex = "status.webhookPath" -// IndexGitProviderWebhookPath indexes a GitProvider by its published status.webhookPath. -func IndexGitProviderWebhookPath(obj client.Object) []string { - provider, ok := obj.(*v1alpha3.GitProvider) - if !ok || provider.Status.WebhookPath == "" { +// IndexGitReceiverWebhookPath indexes a GitReceiver by its published status.webhookPath. +func IndexGitReceiverWebhookPath(obj client.Object) []string { + receiver, ok := obj.(*v1alpha3.GitReceiver) + if !ok || receiver.Status.WebhookPath == "" { return nil } - return []string{provider.Status.WebhookPath} + return []string{receiver.Status.WebhookPath} } // DefaultReceiverMaxRequestBodyBytes caps a delivery's body before its signature is checked. A @@ -82,15 +82,16 @@ const ( // Delivery outcomes, the outcome label of gitopsreverser_receiver_deliveries_total. See // telemetry.ReceiverDeliveriesTotal for what each one means. const ( - deliveryNotified = "notified" - deliveryUnmatched = "unmatched" - deliveryIgnored = "ignored" - deliveryRejected = "rejected" - deliveryInvalid = "invalid" + deliveryNotified = "notified" + deliveryUnknownRepository = "unknown_repository" + deliveryUnknownBranch = "unknown_branch" + deliveryIgnored = "ignored" + deliveryRejected = "rejected" + deliveryInvalid = "invalid" ) -// PushNotifier hands a verified delivery's branch to the branch worker that owns it. It reports -// whether such a worker exists. git.WorkerManager implements it. +// PushNotifier hands a verified delivery's branch to the branch worker that writes it for a +// GitProvider. It reports whether such a worker exists. git.WorkerManager implements it. // // after is the commit the delivery named. It is passed for logging only: a delivery is a hint, // and the worker reads where the branch is from the remote itself rather than trusting it. @@ -100,11 +101,11 @@ type PushNotifier interface { // ReceiverHandlerConfig configures the push receiver. type ReceiverHandlerConfig struct { - // Reader looks up the GitProvider a route names and the Secret holding its token. In the - // manager this is the manager client: GitProviders come from its cache, and Secrets are - // excluded from that cache (Cache.DisableFor), so each delivery reads the token Secret - // directly by name. A rotated token therefore takes effect on the next delivery, and the - // receiver adds no Secret informer. See docs/rbac.md. + // Reader looks up the GitReceiver a route names, the Secret holding its token, and the + // GitProviders a delivery may be for. In the manager this is the manager client: the kinds + // come from its cache, and Secrets are excluded from that cache (Cache.DisableFor), so each + // delivery reads the token Secret directly by name. A rotated token therefore takes effect on + // the next delivery, and the receiver adds no Secret informer. See docs/rbac.md. Reader client.Reader // Notifier receives each verified push to a branch. Required. Notifier PushNotifier @@ -117,20 +118,34 @@ type ReceiverHandlerConfig struct { // ReceiverHandler serves POST /git-push/: the endpoint a Git host's push webhook // points at, so GitOps Reverser learns quickly that somebody pushed to a branch. // -// A delivery is a hint, never a fact. The handler authenticates it, extracts the branch, and asks -// the branch worker to look; the worker then reads the remote's advertisement and decides for -// itself whether the branch gained a foreign commit. Nothing the body says beyond the branch name -// is acted on, so a late, duplicated or replayed delivery costs one ref advertisement and can never -// make the worker believe something the remote does not say. +// One route is one GitReceiver, and one GitReceiver serves every GitProvider in the cluster, so a +// GitHub App or an organization webhook, which has one endpoint for many repositories, needs one +// route. A delivery is routed by what it names: the branch worker of each provider whose repository +// the payload names, for the branch it names, is woken once. +// +// A delivery is a hint, never a fact. The handler authenticates it, extracts the repository and +// the branch, and asks the branch worker to look; the worker then reads the remote's advertisement +// with its own provider's credential and decides for itself whether the branch gained a foreign +// commit. The repository the body names is only compared with the providers' spec.url, never +// fetched, so a delivery cannot choose a URL, a credential or a commit, and a late, duplicated or +// replayed one costs at most one ref advertisement per woken worker (a worker holds one pending +// look, however many deliveries arrive) and can never make a worker believe something the remote +// does not say. +// +// A verified delivery is answered 202 with no body, whatever it woke. Any namespace can hold a +// GitReceiver, and a reply saying what was woken would tell its token holder whether some other +// namespace tracks a repository and branch they name. What a delivery did is on the +// gitopsreverser_receiver_deliveries_total outcome and on each woken GitBranch's +// status.lastPushNoticeAt. // // Authentication comes first. The signature (or GitLab's token) is checked over the raw bytes // before the body is parsed for anything, in constant time, so an unsigned request cannot make a -// target do work or probe which branches are tracked. +// target do work or probe which repositories or branches are tracked. // -// The route does not name the GitProvider. Its digest is keyed by the receiver token, so only +// The route does not name the GitReceiver. Its digest is keyed by the receiver token, so only // whoever holds the token can find it, and every way a request can fail to reach a verified // receiver (no such path, a path from before the token rotated, no token, a wrong signature) -// gets the same 404: the endpoint says nothing about which namespaces or providers exist. +// gets the same 404: the endpoint says nothing about which namespaces or receivers exist. type ReceiverHandler struct { config ReceiverHandlerConfig inFlight chan struct{} @@ -153,21 +168,28 @@ func NewReceiverHandler(config ReceiverHandlerConfig) (*ReceiverHandler, error) return &ReceiverHandler{config: config, inFlight: make(chan struct{}, config.MaxInFlight)}, nil } -// receiverResponse is the body of every 202. matched is 1 when a branch worker was told to look -// and 0 otherwise; it is a number rather than a boolean so a relay fanning one event out to many -// receivers can sum the answers. -type receiverResponse struct { - Matched int `json:"matched"` -} - // pushDelivery is what a verified delivery is acted on for. type pushDelivery struct { - branch string - after string + // repositories is every repository the delivery names, as repourl normalizes them. A host names + // one repository several ways (an HTTPS and an SSH URL, which can differ in host), and a provider + // spelling it any of them is the repository the delivery is for. + repositories map[string]struct{} + branch string + after string } -// errDeliveryIgnored marks a verified delivery that is deliberately not acted on (a tag, another -// repository). It answers 202, not 400: none of those is the sender's mistake. +// names reports whether the delivery names the repository at url. +func (d pushDelivery) names(url string) bool { + normalized := repourl.Normalize(url) + if normalized == "" { + return false + } + _, ok := d.repositories[normalized] + return ok +} + +// errDeliveryIgnored marks a verified delivery that is deliberately not acted on (a tag). It +// answers 202, not 400: that is not the sender's mistake. var errDeliveryIgnored = errors.New("delivery ignored") // ServeHTTP implements http.Handler for push deliveries. @@ -203,15 +225,14 @@ func (h *ReceiverHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - provider, ok := h.lookupProvider(ctx, log, w, path) + receiver, ok := h.lookupReceiver(ctx, log, w, path) if !ok { return } - key := types.NamespacedName{Namespace: provider.Namespace, Name: provider.Name} - receiverType := provider.Spec.Receiver.Type - log = log.WithValues("gitProvider", key.String(), "type", receiverType) + key := receiver.Key() + log = log.WithValues("gitReceiver", key.String(), "type", receiver.Spec.Type) - token, ok := h.lookupToken(ctx, log, w, provider) + token, ok := h.lookupToken(ctx, log, w, receiver) if !ok { return } @@ -219,16 +240,16 @@ func (h *ReceiverHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // the old one must stop serving now, not at the next reconcile. if subtle.ConstantTimeCompare([]byte(v1alpha3.ReceiverPath(token, key.Namespace, key.Name)), []byte(path)) != 1 { log.Info("Push delivery on a path the current receiver token no longer derives; the token was " + - "rotated, so register the provider's new status.webhookPath") + "rotated, so register the GitReceiver's new status.webhookPath") notFound(w) return } - body, ok := h.authenticate(ctx, log, w, r, receiverType, token) + body, ok := h.authenticate(ctx, log, w, r, receiver, token) if !ok { return } - h.act(ctx, log, w, r.Header, key, provider, body) + h.act(ctx, log, w, r.Header, receiver, body) } // authenticate reads the body under the size cap and verifies it, answering the request itself when @@ -239,7 +260,7 @@ func (h *ReceiverHandler) authenticate( log logr.Logger, w http.ResponseWriter, r *http.Request, - receiverType v1alpha3.ReceiverType, + receiver *v1alpha3.GitReceiver, token []byte, ) ([]byte, bool) { body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, h.config.MaxRequestBodyBytes)) @@ -248,7 +269,7 @@ func (h *ReceiverHandler) authenticate( if errors.As(err, &tooLarge) { log.Info("Refused a push delivery over the size cap", "maxBytes", h.config.MaxRequestBodyBytes) http.Error(w, "request body too large", http.StatusRequestEntityTooLarge) - h.record(ctx, receiverType, deliveryRejected) + h.record(ctx, receiver, deliveryRejected) return nil, false } log.V(1).Info("Could not read a push delivery", "error", err.Error()) @@ -256,63 +277,97 @@ func (h *ReceiverHandler) authenticate( return nil, false } - if !verifyDelivery(receiverType, r.Header, body, token) { + if !verifyDelivery(receiver.Spec.Type, r.Header, body, token) { // Info, not V(1): a run of these is a misconfigured hook (a token that does not match) or // somebody probing the endpoint, and either is worth seeing at the default level. Neither // the token nor the body is logged. The answer is the unknown route's, so a caller cannot // tell a wrong signature from a path that serves nothing. log.Info("Rejected a push delivery: signature absent or wrong") notFound(w) - h.record(ctx, receiverType, deliveryRejected) + h.record(ctx, receiver, deliveryRejected) return nil, false } return body, true } // act handles a verified delivery: it decides whether the delivery is a push to a branch and, if -// so, hands the branch to its worker. Every path answers 202 except a body that cannot be read. +// so, wakes the branch workers it names. Every path answers 202 except a body that cannot be read +// and a configuration that cannot be read. func (h *ReceiverHandler) act( ctx context.Context, log logr.Logger, w http.ResponseWriter, header http.Header, - key types.NamespacedName, - provider *v1alpha3.GitProvider, + receiver *v1alpha3.GitReceiver, body []byte, ) { - receiverType := provider.Spec.Receiver.Type + receiverType := receiver.Spec.Type if event, isPush := pushEvent(receiverType, header); !isPush { // A host's ping (GitHub sends one when the hook is created) and any other event the hook // was subscribed to are answered 202, so the host shows the hook as healthy. log.V(1).Info("Ignored a verified delivery that is not a push", "event", event) - h.accept(w, false) - h.record(ctx, receiverType, deliveryIgnored) + accept(w) + h.record(ctx, receiver, deliveryIgnored) return } - delivery, err := parseDelivery(receiverType, jsonBody(header, body), provider.Spec.URL) + delivery, err := parseDelivery(receiverType, jsonBody(header, body)) switch { case errors.Is(err, errDeliveryIgnored): log.V(1).Info("Ignored a verified push delivery", "reason", err.Error()) - h.accept(w, false) - h.record(ctx, receiverType, deliveryIgnored) + accept(w) + h.record(ctx, receiver, deliveryIgnored) return case err != nil: log.Info("Refused a verified push delivery that cannot be read", "error", err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) - h.record(ctx, receiverType, deliveryInvalid) + h.record(ctx, receiver, deliveryInvalid) return } - matched := h.config.Notifier.NotifyPush(key.Namespace, key.Name, delivery.branch, delivery.after) - outcome := deliveryUnmatched - if matched { - outcome = deliveryNotified + matched, outcome, err := h.wake(ctx, delivery) + if err != nil { + log.Error(err, "Could not look up the GitProviders a push delivery is for") + http.Error(w, "receiver not ready", http.StatusServiceUnavailable) + return } log.V(1).Info("Push delivery handled", "branch", delivery.branch, "after", delivery.after, - "outcome", outcome) - h.accept(w, matched) - h.record(ctx, receiverType, outcome) + "outcome", outcome, "matched", matched) + accept(w) + h.record(ctx, receiver, outcome) +} + +// wake tells the branch worker of each GitProvider the delivery is for to look, and returns how many +// were told and the delivery's outcome. A delivery is for every provider, in any namespace, whose +// repository it names. Each provider is asked once, and only the worker writing the branch for it +// answers, so a branch is woken once however many spellings of its repository the delivery carries. +// +// Providers are few, so they are listed from the cache and compared one by one rather than kept in +// an index of normalized URLs. +func (h *ReceiverHandler) wake(ctx context.Context, delivery pushDelivery) (int, string, error) { + var providers v1alpha3.GitProviderList + if err := h.config.Reader.List(ctx, &providers); err != nil { + return 0, "", err + } + named, matched := 0, 0 + for i := range providers.Items { + provider := &providers.Items[i] + if !delivery.names(provider.Spec.URL) { + continue + } + named++ + if h.config.Notifier.NotifyPush(provider.Namespace, provider.Name, delivery.branch, delivery.after) { + matched++ + } + } + switch { + case named == 0: + return 0, deliveryUnknownRepository, nil + case matched == 0: + return 0, deliveryUnknownBranch, nil + default: + return matched, deliveryNotified, nil + } } // receiverRoute returns the path when it is /git-push/. Anything else is no @@ -330,55 +385,49 @@ func receiverRoute(path string) (string, bool) { return path, true } -// lookupProvider finds the GitProvider that published path, through the cache index. A path nobody -// published and a provider without spec.receiver answer the same 404. -func (h *ReceiverHandler) lookupProvider( +// lookupReceiver finds the GitReceiver that published path, through the cache index. A path nobody +// published answers 404. +func (h *ReceiverHandler) lookupReceiver( ctx context.Context, log logr.Logger, w http.ResponseWriter, path string, -) (*v1alpha3.GitProvider, bool) { - var providers v1alpha3.GitProviderList +) (*v1alpha3.GitReceiver, bool) { + var receivers v1alpha3.GitReceiverList if err := h.config.Reader.List( ctx, - &providers, - client.MatchingFields{GitProviderWebhookPathIndex: path}, + &receivers, + client.MatchingFields{GitReceiverWebhookPathIndex: path}, ); err != nil { - log.Error(err, "Could not look up the GitProvider for a push delivery") + log.Error(err, "Could not look up the GitReceiver for a push delivery") http.Error(w, "receiver not ready", http.StatusServiceUnavailable) return nil, false } - if len(providers.Items) != 1 { - if len(providers.Items) > 1 { + if len(receivers.Items) != 1 { + if len(receivers.Items) > 1 { // Two digests keyed by tokens agree only if a status was written by hand. - log.Info("Push delivery path is published by more than one GitProvider; serving none", - "count", len(providers.Items)) + log.Info("Push delivery path is published by more than one GitReceiver; serving none", + "count", len(receivers.Items)) } notFound(w) return nil, false } - provider := &providers.Items[0] - if provider.Spec.Receiver == nil { - log.V(1).Info("Push delivery for a GitProvider whose spec.receiver was removed", - "gitProvider", provider.Namespace+"/"+provider.Name) - notFound(w) - return nil, false - } - return provider, true + return &receivers.Items[0], true } -// lookupToken reads the receiver token from the Secret spec.receiver.secretRef names. A missing -// Secret or key answers the unknown route's 404: the path was derived from a token that is gone, -// so nothing can be verified on it. Only a read that failed answers 503, so a host retries it. +// lookupToken reads the receiver token from the Secret spec.secretRef names, in the GitReceiver's +// namespace. A missing Secret or key answers the unknown route's 404: the path was derived from a +// token that is gone, so nothing can be verified on it. Only a read that failed answers 503, so a +// host retries it. func (h *ReceiverHandler) lookupToken( ctx context.Context, log logr.Logger, w http.ResponseWriter, - provider *v1alpha3.GitProvider, + receiver *v1alpha3.GitReceiver, ) ([]byte, bool) { - secretName := provider.Spec.Receiver.SecretRef.Name + secretName := receiver.Spec.SecretRef.Name var secret corev1.Secret - err := h.config.Reader.Get(ctx, types.NamespacedName{Namespace: provider.Namespace, Name: secretName}, &secret) + err := h.config.Reader.Get(ctx, types.NamespacedName{Namespace: receiver.Namespace, Name: secretName}, &secret) if err != nil { if apierrors.IsNotFound(err) { log.Info("Push receiver token Secret not found", "secret", secretName) @@ -472,10 +521,31 @@ func pushEvent(receiverType v1alpha3.ReceiverType, header http.Header) (string, } // hostPushPayload is the part of a GitHub, GitLab or Gitea push event the receiver reads. The -// three hosts agree on these two fields, which is why one shape serves all of them. +// three hosts agree on ref and after. They name the repository in different fields, so every field +// one of them fills with a clone or web URL is read: GitHub and Gitea fill repository.clone_url, +// ssh_url and html_url (GitHub also git_url); GitLab fills project.git_http_url, git_ssh_url and +// web_url, and the same in its legacy repository object. The API URLs (repository.url on Gitea, a +// homepage on GitHub) are not read: they name no clone URL a provider could spell. type hostPushPayload struct { - Ref string `json:"ref"` - After string `json:"after"` + Ref string `json:"ref"` + After string `json:"after"` + Repository hostRepository `json:"repository"` + Project hostRepository `json:"project"` +} + +// hostRepository is the repository a push event names, in any of the hosts' spellings. +type hostRepository struct { + CloneURL string `json:"clone_url"` + SSHURL string `json:"ssh_url"` + GitURL string `json:"git_url"` + HTMLURL string `json:"html_url"` + GitHTTPURL string `json:"git_http_url"` + GitSSHURL string `json:"git_ssh_url"` + WebURL string `json:"web_url"` +} + +func (r hostRepository) urls() []string { + return []string{r.CloneURL, r.SSHURL, r.GitURL, r.HTMLURL, r.GitHTTPURL, r.GitSSHURL, r.WebURL} } // genericPushPayload is a generic-hmac relay's body. @@ -485,19 +555,6 @@ type genericPushPayload struct { After string `json:"after"` } -// parseDelivery extracts the branch a verified push names. It returns errDeliveryIgnored (wrapped, -// with the reason) for a push that is deliberately not acted on, and any other error for a body -// that cannot be read. -// -// after is optional in every shape. It never decides whether the branch moved (it only spares an open -// window a notice for the tip already observed), so refusing a delivery for lacking it would turn a -// field nothing depends on into a reason for a host to disable the hook. -// -// The repository check is generic-hmac's alone. A host's own hook is configured on one repository -// (or an organization), and a delivery from a sibling repository on an organization hook costs one -// ref advertisement for a branch of the same name; comparing the host's repository URLs with -// spec.url instead would silently drop every delivery for a provider that reaches the host by -// another name (an SSH host alias, ssh.github.com:443), which is the worse failure. // jsonBody is the JSON document a delivery carries. GitHub's default content type for a webhook is // application/x-www-form-urlencoded, which wraps the same JSON in a payload field; the signature is // over the raw body, so this unwrapping happens only after it was verified. Flux accepts that form @@ -514,9 +571,17 @@ func jsonBody(header http.Header, body []byte) []byte { return []byte(form.Get("payload")) } -func parseDelivery(receiverType v1alpha3.ReceiverType, body []byte, providerURL string) (pushDelivery, error) { +// parseDelivery extracts the repository and the branch a verified push names. It returns +// errDeliveryIgnored (wrapped, with the reason) for a push that is deliberately not acted on, and +// any other error for a body that cannot be read. A delivery that names no repository cannot be +// routed, so it is unreadable rather than ignored. +// +// after is optional in every shape. It never decides whether the branch moved (it only spares an open +// window a notice for the tip already observed), so refusing a delivery for lacking it would turn a +// field nothing depends on into a reason for a host to disable the hook. +func parseDelivery(receiverType v1alpha3.ReceiverType, body []byte) (pushDelivery, error) { if receiverType == v1alpha3.ReceiverTypeGenericHMAC { - return parseGenericDelivery(body, providerURL) + return parseGenericDelivery(body) } var payload hostPushPayload if err := json.Unmarshal(body, &payload); err != nil { @@ -529,13 +594,15 @@ func parseDelivery(receiverType v1alpha3.ReceiverType, body []byte, providerURL if err != nil { return pushDelivery{}, err } - return pushDelivery{branch: branch, after: payload.After}, nil + repositories := normalizedRepositories(append(payload.Repository.urls(), payload.Project.urls()...)) + if len(repositories) == 0 { + return pushDelivery{}, errors.New("push event names no repository") + } + return pushDelivery{repositories: repositories, branch: branch, after: payload.After}, nil } -// parseGenericDelivery reads a generic-hmac body. Its branch may be a bare name or a full ref; a -// repository other than spec.url is ignored, not refused, because a relay fanning one host's -// events out to every receiver is doing exactly what it is for. -func parseGenericDelivery(body []byte, providerURL string) (pushDelivery, error) { +// parseGenericDelivery reads a generic-hmac body. Its branch may be a bare name or a full ref. +func parseGenericDelivery(body []byte) (pushDelivery, error) { var payload genericPushPayload if err := json.Unmarshal(body, &payload); err != nil { return pushDelivery{}, fmt.Errorf("body is not valid JSON: %w", err) @@ -543,6 +610,10 @@ func parseGenericDelivery(body []byte, providerURL string) (pushDelivery, error) if payload.Branch == "" { return pushDelivery{}, errors.New("body has no branch") } + repositories := normalizedRepositories([]string{payload.Repository}) + if len(repositories) == 0 { + return pushDelivery{}, errors.New("body has no repository") + } branch := payload.Branch if strings.HasPrefix(branch, "refs/") { var err error @@ -550,10 +621,19 @@ func parseGenericDelivery(body []byte, providerURL string) (pushDelivery, error) return pushDelivery{}, err } } - if payload.Repository != "" && !repourl.Same(payload.Repository, providerURL) { - return pushDelivery{}, fmt.Errorf("%w: names another repository", errDeliveryIgnored) + return pushDelivery{repositories: repositories, branch: branch, after: payload.After}, nil +} + +// normalizedRepositories is the set of repositories urls name, as repourl normalizes them. An empty +// URL names none. +func normalizedRepositories(urls []string) map[string]struct{} { + repositories := map[string]struct{}{} + for _, raw := range urls { + if normalized := repourl.Normalize(raw); normalized != "" { + repositories[normalized] = struct{}{} + } } - return pushDelivery{branch: branch, after: payload.After}, nil + return repositories } // branchFromRef returns the branch a full ref names. A ref outside refs/heads/ (a tag, a pull @@ -575,24 +655,20 @@ func notFound(w http.ResponseWriter) { http.Error(w, "no push receiver at this path", http.StatusNotFound) } -// accept answers a verified delivery. -func (h *ReceiverHandler) accept(w http.ResponseWriter, matched bool) { - resp := receiverResponse{} - if matched { - resp.Matched = 1 - } - w.Header().Set("Content-Type", "application/json") +// accept answers a verified delivery: 202, with no body (see ReceiverHandler). +func accept(w http.ResponseWriter) { w.WriteHeader(http.StatusAccepted) - _ = json.NewEncoder(w).Encode(resp) } -// record counts one delivery for a provider with a receiver. -func (h *ReceiverHandler) record(ctx context.Context, receiverType v1alpha3.ReceiverType, outcome string) { +// record counts one delivery to a GitReceiver. +func (h *ReceiverHandler) record(ctx context.Context, receiver *v1alpha3.GitReceiver, outcome string) { if telemetry.ReceiverDeliveriesTotal == nil { return } telemetry.ReceiverDeliveriesTotal.Add(ctx, 1, metric.WithAttributes( - attribute.String("type", string(receiverType)), + attribute.String("receiver_namespace", receiver.Namespace), + attribute.String("receiver_name", receiver.Name), + attribute.String("type", string(receiver.Spec.Type)), attribute.String("outcome", outcome), )) } diff --git a/internal/webhook/receiver_test.go b/internal/webhook/receiver_test.go index 39c96204..aa335ad9 100644 --- a/internal/webhook/receiver_test.go +++ b/internal/webhook/receiver_test.go @@ -31,13 +31,16 @@ import ( ) const ( - receiverTestNamespace = "team-a" + receiverTestNamespace = "platform" receiverTestToken = "s3cr3t-token" - receiverTestRepoURL = "https://github.com/acme/platform.git" receiverTestCommit = "4f2c1ab9e0d1c2b3a4f5e6d7c8b9a0f1e2d3c4b5" + + platformRepo = "https://github.com/acme/platform.git" + tenantRepo = "https://github.com/acme/tenant.git" ) -// fakePushNotifier records every NotifyPush call and reports a worker for the branches in workers. +// fakePushNotifier records every NotifyPush call and reports a worker for the "/ +// @" keys in workers. type fakePushNotifier struct { workers map[string]bool calls []fakePushCall @@ -49,37 +52,53 @@ type fakePushCall struct { func (f *fakePushNotifier) NotifyPush(providerNamespace, providerName, branch, after string) bool { f.calls = append(f.calls, fakePushCall{providerNamespace, providerName, branch, after}) - return f.workers[branch] + return f.workers[providerNamespace+"/"+providerName+"@"+branch] } -// receiverTestProvider is a provider whose status publishes the path receiverTestToken derives, as -// the GitProvider controller would. -func receiverTestProvider(name string, receiver *v1alpha3.GitProviderReceiver) *v1alpha3.GitProvider { - return &v1alpha3.GitProvider{ +// testWorkers is the branch workers the handler tests run against: main of each provider's +// repository is written, through the provider listed here. +func testWorkers() map[string]bool { + return map[string]bool{ + "platform/gh-config@main": true, + "platform/gt-config@main": true, + "tenant-a/tenant-config@main": true, + } +} + +// receiverTestReceiver is a GitReceiver in the test namespace whose status publishes the path +// receiverTestToken derives, as the GitReceiver controller would. +func receiverTestReceiver(name string, receiverType v1alpha3.ReceiverType, secret string) *v1alpha3.GitReceiver { + return &v1alpha3.GitReceiver{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: receiverTestNamespace}, - Spec: v1alpha3.GitProviderSpec{ - URL: receiverTestRepoURL, - AllowedBranches: []string{"*"}, - Receiver: receiver, + Spec: v1alpha3.GitReceiverSpec{ + Type: receiverType, + SecretRef: meta.LocalObjectReference{Name: secret}, }, - Status: v1alpha3.GitProviderStatus{WebhookPath: route(name)}, + Status: v1alpha3.GitReceiverStatus{WebhookPath: route(name)}, } } -// route is the path receiverTestToken derives for the provider name in the test namespace. +// route is the path receiverTestToken derives for the receiver name in the test namespace. func route(name string) string { return v1alpha3.ReceiverPath([]byte(receiverTestToken), receiverTestNamespace, name) } -// rotatedProvider still publishes the path a token it no longer holds derived. -func rotatedProvider() *v1alpha3.GitProvider { - provider := receiverTestProvider("rotated", receiverOf(v1alpha3.ReceiverTypeGitHub, "receiver-token")) - provider.Status.WebhookPath = v1alpha3.ReceiverPath([]byte("the-old-token"), receiverTestNamespace, "rotated") - return provider +// rotatedReceiver still publishes the path a token it no longer holds derived. +func rotatedReceiver() *v1alpha3.GitReceiver { + receiver := receiverTestReceiver("rotated", v1alpha3.ReceiverTypeGitHub, "receiver-token") + receiver.Status.WebhookPath = v1alpha3.ReceiverPath([]byte("the-old-token"), receiverTestNamespace, "rotated") + return receiver } -func receiverOf(receiverType v1alpha3.ReceiverType, secret string) *v1alpha3.GitProviderReceiver { - return &v1alpha3.GitProviderReceiver{Type: receiverType, SecretRef: meta.LocalObjectReference{Name: secret}} +// testProvider is a GitProvider for url. +func testProvider(namespace, name, url string) *v1alpha3.GitProvider { + return &v1alpha3.GitProvider{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: v1alpha3.GitProviderSpec{ + URL: url, + AllowedBranches: []string{"*"}, + }, + } } func receiverTestSecret(name string, data map[string][]byte) *corev1.Secret { @@ -89,20 +108,27 @@ func receiverTestSecret(name string, data map[string][]byte) *corev1.Secret { } } -// receiverTestObjects is one provider per receiver type sharing one token Secret, plus the -// providers that must not serve or cannot verify yet. +// receiverTestObjects is one receiver per type sharing one token Secret, the receivers that cannot +// verify yet, and the providers every one of them routes to by URL. +// +// The platform repository has two providers spelling it two ways (only gh-config writes main), the +// tenant repository one provider in another namespace, and the Gitea repository one provider +// spelling it by Gitea's SSH host. func receiverTestObjects() []client.Object { return []client.Object{ receiverTestSecret("receiver-token", map[string][]byte{v1alpha3.ReceiverTokenKey: []byte(receiverTestToken)}), receiverTestSecret("wrong-key", map[string][]byte{"password": []byte(receiverTestToken)}), - receiverTestProvider("gh", receiverOf(v1alpha3.ReceiverTypeGitHub, "receiver-token")), - receiverTestProvider("gl", receiverOf(v1alpha3.ReceiverTypeGitLab, "receiver-token")), - receiverTestProvider("gt", receiverOf(v1alpha3.ReceiverTypeGitea, "receiver-token")), - receiverTestProvider("gen", receiverOf(v1alpha3.ReceiverTypeGenericHMAC, "receiver-token")), - receiverTestProvider("plain", nil), - receiverTestProvider("no-secret", receiverOf(v1alpha3.ReceiverTypeGitHub, "absent")), - receiverTestProvider("no-key", receiverOf(v1alpha3.ReceiverTypeGitHub, "wrong-key")), - rotatedProvider(), + receiverTestReceiver("gh", v1alpha3.ReceiverTypeGitHub, "receiver-token"), + receiverTestReceiver("gl", v1alpha3.ReceiverTypeGitLab, "receiver-token"), + receiverTestReceiver("gt", v1alpha3.ReceiverTypeGitea, "receiver-token"), + receiverTestReceiver("gen", v1alpha3.ReceiverTypeGenericHMAC, "receiver-token"), + receiverTestReceiver("no-secret", v1alpha3.ReceiverTypeGitHub, "absent"), + receiverTestReceiver("no-key", v1alpha3.ReceiverTypeGitHub, "wrong-key"), + rotatedReceiver(), + testProvider(receiverTestNamespace, "gh-config", platformRepo), + testProvider(receiverTestNamespace, "ssh-config", "git@github.com:acme/platform"), + testProvider("tenant-a", "tenant-config", tenantRepo), + testProvider(receiverTestNamespace, "gt-config", "ssh://git@gitea-ssh.example/acme/platform.git"), } } @@ -117,7 +143,7 @@ func newReceiverTestHandler( require.NoError(t, clientgoscheme.AddToScheme(scheme)) require.NoError(t, v1alpha3.AddToScheme(scheme)) builder := fake.NewClientBuilder().WithScheme(scheme).WithObjects(receiverTestObjects()...). - WithIndex(&v1alpha3.GitProvider{}, GitProviderWebhookPathIndex, IndexGitProviderWebhookPath) + WithIndex(&v1alpha3.GitReceiver{}, GitReceiverWebhookPathIndex, IndexGitReceiverWebhookPath) if funcs != nil { builder = builder.WithInterceptorFuncs(*funcs) } @@ -136,9 +162,35 @@ func hmacHex(body, token string) string { return hex.EncodeToString(mac.Sum(nil)) } -func hostPushBody(ref string) string { - return `{"ref":"` + ref + `","before":"0000","after":"` + receiverTestCommit + `",` + - `"repository":{"clone_url":"https://github.com/acme/platform.git"}}` +// hostPushBody is a GitHub or Gitea push event for ref of the repository cloned from cloneURL. +func hostPushBody(ref, cloneURL string) string { + body, _ := json.Marshal(map[string]any{ + "ref": ref, "before": "0000", "after": receiverTestCommit, + "repository": map[string]string{"clone_url": cloneURL, "html_url": strings.TrimSuffix(cloneURL, ".git")}, + }) + return string(body) +} + +// gitlabPushBody is a GitLab push event for ref, which names its repository in the project object. +func gitlabPushBody(ref, httpURL string) string { + body, _ := json.Marshal(map[string]any{ + "ref": ref, "after": receiverTestCommit, + "project": map[string]string{"git_http_url": httpURL, "web_url": strings.TrimSuffix(httpURL, ".git")}, + }) + return string(body) +} + +// giteaPushBody is a Gitea push event whose HTTP and SSH URLs name different hosts, as a Gitea with +// a separate SSH_DOMAIN sends them. +func giteaPushBody(ref string) string { + body, _ := json.Marshal(map[string]any{ + "ref": ref, "after": receiverTestCommit, + "repository": map[string]string{ + "clone_url": "http://gitea-http.example:3000/acme/platform.git", + "ssh_url": "git@gitea-ssh.example:acme/platform.git", + }, + }) + return string(body) } func genericBody(repository, branch string) string { @@ -172,34 +224,53 @@ type receiverCase struct { body string wantStatus int - // wantMatched is the 202 body's matched value; ignored for any other status. - wantMatched int - // wantCall is the NotifyPush the delivery must make; nil asserts none was made. - wantCall *fakePushCall - // wantType/wantOutcome is the one delivery the metric must count; empty asserts none. - wantType string - wantOutcome string + // wantCalls is every NotifyPush the delivery must make, in any order; nil asserts none was made. + wantCalls []fakePushCall + // wantReceiver/wantOutcome is the one delivery the metric must count; empty asserts none. + wantReceiver string + wantType string + wantOutcome string +} + +func call(namespace, name, branch string) fakePushCall { + return fakePushCall{namespace, name, branch, receiverTestCommit} } func receiverCases() map[string]receiverCase { - mainPush := hostPushBody("refs/heads/main") - otherPush := hostPushBody("refs/heads/feature/x") - tagPush := hostPushBody("refs/tags/v1.0.0") - ghCall := &fakePushCall{receiverTestNamespace, "gh", "main", receiverTestCommit} + mainPush := hostPushBody("refs/heads/main", platformRepo) + otherPush := hostPushBody("refs/heads/feature/x", platformRepo) + tagPush := hostPushBody("refs/tags/v1.0.0", platformRepo) + tenantPush := hostPushBody("refs/heads/main", tenantRepo) + unknownPush := hostPushBody("refs/heads/main", "https://github.com/acme/other.git") + ghPlatform := []fakePushCall{call(receiverTestNamespace, "gh-config", "main"), + call(receiverTestNamespace, "ssh-config", "main")} genMain := genericBody("git@github.com:acme/platform", "main") + gitlabMain := gitlabPushBody("refs/heads/main", platformRepo) return map[string]receiverCase{ - // github - "github push to a worked branch is notified": { + // github, and the routing every type shares + "a push wakes the worker of the provider that writes the branch, once": { path: route("gh"), headers: githubHeaders("push", mainPush, receiverTestToken), body: mainPush, - wantStatus: http.StatusAccepted, wantMatched: 1, wantCall: ghCall, - wantType: "github", wantOutcome: deliveryNotified, + wantStatus: http.StatusAccepted, wantCalls: ghPlatform, + wantReceiver: "gh", wantType: "github", wantOutcome: deliveryNotified, }, - "github push to a branch nothing writes is still a success": { + "a push wakes a provider in any namespace": { + path: route("gh"), headers: githubHeaders("push", tenantPush, receiverTestToken), body: tenantPush, + wantStatus: http.StatusAccepted, + wantCalls: []fakePushCall{call("tenant-a", "tenant-config", "main")}, + wantReceiver: "gh", wantType: "github", wantOutcome: deliveryNotified, + }, + "a push for a repository no provider has is still a success": { + path: route("gh"), headers: githubHeaders("push", unknownPush, receiverTestToken), body: unknownPush, + wantStatus: http.StatusAccepted, + wantReceiver: "gh", wantType: "github", wantOutcome: deliveryUnknownRepository, + }, + "a push to a branch nothing writes is still a success": { path: route("gh"), headers: githubHeaders("push", otherPush, receiverTestToken), body: otherPush, - wantStatus: http.StatusAccepted, wantMatched: 0, - wantCall: &fakePushCall{receiverTestNamespace, "gh", "feature/x", receiverTestCommit}, - wantType: "github", wantOutcome: deliveryUnmatched, + wantStatus: http.StatusAccepted, + wantCalls: []fakePushCall{call(receiverTestNamespace, "gh-config", "feature/x"), + call(receiverTestNamespace, "ssh-config", "feature/x")}, + wantReceiver: "gh", wantType: "github", wantOutcome: deliveryUnknownBranch, }, // GitHub's default content type wraps the JSON in a payload field, and signs the raw body. "github form-encoded push reaches the worker": { @@ -210,17 +281,16 @@ func receiverCases() map[string]receiverCase { return h }(), body: formEncoded(mainPush), - wantStatus: http.StatusAccepted, wantMatched: 1, - wantCall: &fakePushCall{receiverTestNamespace, "gh", "main", receiverTestCommit}, - wantType: "github", wantOutcome: deliveryNotified, + wantStatus: http.StatusAccepted, wantCalls: ghPlatform, + wantReceiver: "gh", wantType: "github", wantOutcome: deliveryNotified, }, "github wrong signature is rejected": { path: route("gh"), headers: githubHeaders("push", mainPush, "not-the-token"), body: mainPush, - wantStatus: http.StatusNotFound, wantType: "github", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryRejected, }, "github missing signature is rejected": { path: route("gh"), headers: map[string]string{headerGitHubEvent: "push"}, body: mainPush, - wantStatus: http.StatusNotFound, wantType: "github", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryRejected, }, "github signature without the sha256 prefix is rejected": { path: route("gh"), @@ -228,7 +298,7 @@ func receiverCases() map[string]receiverCase { headerGitHubEvent: "push", headerGitHubSignature: hmacHex(mainPush, receiverTestToken), }, body: mainPush, - wantStatus: http.StatusNotFound, wantType: "github", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryRejected, }, "github signature that is not hex is rejected": { path: route("gh"), @@ -236,67 +306,75 @@ func receiverCases() map[string]receiverCase { headerGitHubEvent: "push", headerGitHubSignature: "sha256=zz", }, body: mainPush, - wantStatus: http.StatusNotFound, wantType: "github", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryRejected, }, "github ping is accepted and does nothing": { path: route("gh"), headers: githubHeaders("ping", `{"zen":"hi"}`, receiverTestToken), body: `{"zen":"hi"}`, - wantStatus: http.StatusAccepted, wantType: "github", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryIgnored, }, "github non-push event is ignored": { path: route("gh"), headers: githubHeaders("issues", `{}`, receiverTestToken), body: `{}`, - wantStatus: http.StatusAccepted, wantType: "github", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryIgnored, }, "github tag push is ignored": { path: route("gh"), headers: githubHeaders("push", tagPush, receiverTestToken), body: tagPush, - wantStatus: http.StatusAccepted, wantType: "github", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryIgnored, }, "github verified body that is not JSON is invalid": { path: route("gh"), headers: githubHeaders("push", "not json", receiverTestToken), body: "not json", - wantStatus: http.StatusBadRequest, wantType: "github", wantOutcome: deliveryInvalid, + wantStatus: http.StatusBadRequest, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryInvalid, }, "github verified push without a ref is invalid": { path: route("gh"), headers: githubHeaders("push", `{"after":"abc"}`, receiverTestToken), body: `{"after":"abc"}`, - wantStatus: http.StatusBadRequest, wantType: "github", wantOutcome: deliveryInvalid, + wantStatus: http.StatusBadRequest, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryInvalid, }, "github ref naming no branch is invalid": { path: route("gh"), headers: githubHeaders("push", `{"ref":"refs/heads/"}`, receiverTestToken), body: `{"ref":"refs/heads/"}`, - wantStatus: http.StatusBadRequest, wantType: "github", wantOutcome: deliveryInvalid, + wantStatus: http.StatusBadRequest, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryInvalid, + }, + "github push naming no repository is invalid": { + path: route("gh"), + headers: githubHeaders("push", `{"ref":"refs/heads/main"}`, receiverTestToken), + body: `{"ref":"refs/heads/main"}`, + wantStatus: http.StatusBadRequest, wantReceiver: "gh", wantType: "github", wantOutcome: deliveryInvalid, }, // gitlab - "gitlab push with the token is notified": { - path: route("gl"), headers: gitlabHeaders("Push Hook", receiverTestToken), body: mainPush, - wantStatus: http.StatusAccepted, wantMatched: 1, - wantCall: &fakePushCall{receiverTestNamespace, "gl", "main", receiverTestCommit}, - wantType: "gitlab", wantOutcome: deliveryNotified, + "gitlab push names its repository in the project and is notified": { + path: route("gl"), headers: gitlabHeaders("Push Hook", receiverTestToken), body: gitlabMain, + wantStatus: http.StatusAccepted, + wantCalls: ghPlatform, + wantReceiver: "gl", wantType: "gitlab", wantOutcome: deliveryNotified, }, "gitlab wrong token is rejected": { - path: route("gl"), headers: gitlabHeaders("Push Hook", "nope"), body: mainPush, - wantStatus: http.StatusNotFound, wantType: "gitlab", wantOutcome: deliveryRejected, + path: route("gl"), headers: gitlabHeaders("Push Hook", "nope"), body: gitlabMain, + wantStatus: http.StatusNotFound, wantReceiver: "gl", wantType: "gitlab", wantOutcome: deliveryRejected, }, "gitlab missing token is rejected": { - path: route("gl"), headers: map[string]string{headerGitLabEvent: "Push Hook"}, body: mainPush, - wantStatus: http.StatusNotFound, wantType: "gitlab", wantOutcome: deliveryRejected, + path: route("gl"), headers: map[string]string{headerGitLabEvent: "Push Hook"}, body: gitlabMain, + wantStatus: http.StatusNotFound, wantReceiver: "gl", wantType: "gitlab", wantOutcome: deliveryRejected, }, "gitlab tag push hook is ignored": { path: route("gl"), headers: gitlabHeaders("Tag Push Hook", receiverTestToken), body: tagPush, - wantStatus: http.StatusAccepted, wantType: "gitlab", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gl", wantType: "gitlab", wantOutcome: deliveryIgnored, }, // gitea - "gitea push is notified": { - path: route("gt"), headers: giteaHeaders("push", mainPush, receiverTestToken), body: mainPush, - wantStatus: http.StatusAccepted, wantMatched: 1, - wantCall: &fakePushCall{receiverTestNamespace, "gt", "main", receiverTestCommit}, - wantType: "gitea", wantOutcome: deliveryNotified, + "gitea push reaches a provider spelling the repository by its SSH host": { + path: route("gt"), + headers: giteaHeaders("push", giteaPushBody("refs/heads/main"), receiverTestToken), + body: giteaPushBody("refs/heads/main"), + wantStatus: http.StatusAccepted, + wantCalls: []fakePushCall{call(receiverTestNamespace, "gt-config", "main")}, + wantReceiver: "gt", wantType: "gitea", wantOutcome: deliveryNotified, }, "gitea wrong signature is rejected": { path: route("gt"), headers: giteaHeaders("push", mainPush, "nope"), body: mainPush, - wantStatus: http.StatusNotFound, wantType: "gitea", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gt", wantType: "gitea", wantOutcome: deliveryRejected, }, "gitea github-style prefixed signature is rejected": { path: route("gt"), @@ -304,49 +382,62 @@ func receiverCases() map[string]receiverCase { headerGiteaEvent: "push", headerGiteaSignature: "sha256=" + hmacHex(mainPush, receiverTestToken), }, body: mainPush, - wantStatus: http.StatusNotFound, wantType: "gitea", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gt", wantType: "gitea", wantOutcome: deliveryRejected, }, "gitea non-push event is ignored": { path: route("gt"), headers: giteaHeaders("create", `{}`, receiverTestToken), body: `{}`, - wantStatus: http.StatusAccepted, wantType: "gitea", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gt", wantType: "gitea", wantOutcome: deliveryIgnored, }, // generic-hmac "generic-hmac push for the same repository spelled differently is notified": { path: route("gen"), headers: genericHeaders(genMain, receiverTestToken), body: genMain, - wantStatus: http.StatusAccepted, wantMatched: 1, - wantCall: &fakePushCall{receiverTestNamespace, "gen", "main", receiverTestCommit}, - wantType: "generic-hmac", wantOutcome: deliveryNotified, + wantStatus: http.StatusAccepted, + wantCalls: ghPlatform, + wantReceiver: "gen", wantType: "generic-hmac", wantOutcome: deliveryNotified, }, "generic-hmac full ref is read as its branch": { - path: route("gen"), - headers: genericHeaders(genericBody("", "refs/heads/main"), receiverTestToken), - body: genericBody("", "refs/heads/main"), - wantStatus: http.StatusAccepted, wantMatched: 1, - wantCall: &fakePushCall{receiverTestNamespace, "gen", "main", receiverTestCommit}, - wantType: "generic-hmac", wantOutcome: deliveryNotified, - }, - "generic-hmac another repository is ignored, not refused": { + path: route("gen"), + headers: genericHeaders(genericBody(platformRepo, "refs/heads/main"), receiverTestToken), + body: genericBody(platformRepo, "refs/heads/main"), + wantStatus: http.StatusAccepted, + wantCalls: ghPlatform, + wantReceiver: "gen", wantType: "generic-hmac", wantOutcome: deliveryNotified, + }, + "generic-hmac another repository is counted and ignored": { path: route("gen"), headers: genericHeaders(genericBody("https://github.com/acme/other", "main"), receiverTestToken), body: genericBody("https://github.com/acme/other", "main"), - wantStatus: http.StatusAccepted, wantType: "generic-hmac", wantOutcome: deliveryIgnored, + wantStatus: http.StatusAccepted, wantReceiver: "gen", wantType: "generic-hmac", + wantOutcome: deliveryUnknownRepository, }, "generic-hmac tag ref is ignored": { - path: route("gen"), - headers: genericHeaders(genericBody("", "refs/tags/v1"), receiverTestToken), - body: genericBody("", "refs/tags/v1"), - wantStatus: http.StatusAccepted, wantType: "generic-hmac", wantOutcome: deliveryIgnored, + path: route("gen"), + headers: genericHeaders(genericBody(platformRepo, "refs/tags/v1"), receiverTestToken), + body: genericBody(platformRepo, "refs/tags/v1"), + wantStatus: http.StatusAccepted, + wantReceiver: "gen", + wantType: "generic-hmac", + wantOutcome: deliveryIgnored, }, "generic-hmac wrong signature is rejected": { path: route("gen"), headers: genericHeaders(genMain, "nope"), body: genMain, - wantStatus: http.StatusNotFound, wantType: "generic-hmac", wantOutcome: deliveryRejected, + wantStatus: http.StatusNotFound, wantReceiver: "gen", wantType: "generic-hmac", + wantOutcome: deliveryRejected, }, "generic-hmac without a branch is invalid": { path: route("gen"), headers: genericHeaders(`{"repository":"x"}`, receiverTestToken), body: `{"repository":"x"}`, - wantStatus: http.StatusBadRequest, wantType: "generic-hmac", wantOutcome: deliveryInvalid, + wantStatus: http.StatusBadRequest, wantReceiver: "gen", wantType: "generic-hmac", + wantOutcome: deliveryInvalid, + }, + "generic-hmac without a repository is invalid": { + path: route("gen"), + headers: genericHeaders(genericBody("", "main"), receiverTestToken), + body: genericBody("", "main"), + wantStatus: http.StatusBadRequest, wantReceiver: "gen", wantType: "generic-hmac", + wantOutcome: deliveryInvalid, }, // every way of reaching no verified receiver is the same 404 @@ -356,12 +447,8 @@ func receiverCases() map[string]receiverCase { body: mainPush, wantStatus: http.StatusNotFound, }, - "a provider without spec.receiver is 404": { - path: route("plain"), headers: githubHeaders("push", mainPush, receiverTestToken), body: mainPush, - wantStatus: http.StatusNotFound, - }, "a path from before the token rotated is 404": { - path: rotatedProvider().Status.WebhookPath, + path: rotatedReceiver().Status.WebhookPath, headers: githubHeaders("push", mainPush, receiverTestToken), body: mainPush, wantStatus: http.StatusNotFound, @@ -375,7 +462,7 @@ func receiverCases() map[string]receiverCase { body: mainPush, wantStatus: http.StatusNotFound, }, "the old namespace/name route is 404": { - path: "/git-push/team-a/" + "gh", + path: "/git-push/platform/" + "gh", headers: githubHeaders("push", mainPush, receiverTestToken), body: mainPush, wantStatus: http.StatusNotFound, @@ -405,7 +492,7 @@ func TestReceiverHandler_ServeHTTP(t *testing.T) { reader, err := telemetry.InitTestExporter() require.NoError(t, err) - notifier := &fakePushNotifier{workers: map[string]bool{"main": true}} + notifier := &fakePushNotifier{workers: testWorkers()} h := newReceiverTestHandler(t, notifier, 0, nil) method := tc.method @@ -421,16 +508,13 @@ func TestReceiverHandler_ServeHTTP(t *testing.T) { require.Equal(t, tc.wantStatus, rec.Code, "body: %s", rec.Body.String()) if tc.wantStatus == http.StatusAccepted { - var resp receiverResponse - require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) - assert.Equal(t, tc.wantMatched, resp.Matched) - assert.Equal(t, "application/json", rec.Header().Get("Content-Type")) + assert.Empty(t, rec.Body.String(), "a 202 says nothing about what the delivery woke") } - if tc.wantCall == nil { + if tc.wantCalls == nil { assert.Empty(t, notifier.calls, "a delivery that is not acted on must not reach a worker") } else { - assert.Equal(t, []fakePushCall{*tc.wantCall}, notifier.calls) + assert.ElementsMatch(t, tc.wantCalls, notifier.calls) } total, _ := telemetry.CollectInt64Sum(reader, "gitopsreverser_receiver_deliveries_total", nil) @@ -440,8 +524,11 @@ func TestReceiverHandler_ServeHTTP(t *testing.T) { } assert.Equal(t, int64(1), total, "exactly one delivery is counted") got, ok := telemetry.CollectInt64Sum(reader, "gitopsreverser_receiver_deliveries_total", - map[string]string{"type": tc.wantType, "outcome": tc.wantOutcome}) - require.True(t, ok, "no %s/%s delivery recorded", tc.wantType, tc.wantOutcome) + map[string]string{ + "receiver_namespace": receiverTestNamespace, "receiver_name": tc.wantReceiver, + "type": tc.wantType, "outcome": tc.wantOutcome, + }) + require.True(t, ok, "no %s %s/%s delivery recorded", tc.wantReceiver, tc.wantType, tc.wantOutcome) assert.Equal(t, int64(1), got) }) } @@ -453,10 +540,10 @@ func TestReceiverHandler_OversizedBodyIsRefusedBeforeVerification(t *testing.T) reader, err := telemetry.InitTestExporter() require.NoError(t, err) - notifier := &fakePushNotifier{workers: map[string]bool{"main": true}} + notifier := &fakePushNotifier{workers: testWorkers()} h := newReceiverTestHandler(t, notifier, 64, nil) - body := hostPushBody("refs/heads/main") + body := hostPushBody("refs/heads/main", platformRepo) require.Greater(t, len(body), 64) req := httptest.NewRequest(http.MethodPost, route("gh"), strings.NewReader(body)) for k, v := range githubHeaders("push", body, receiverTestToken) { @@ -474,36 +561,60 @@ func TestReceiverHandler_OversizedBodyIsRefusedBeforeVerification(t *testing.T) } // TestReceiverHandler_UnreadableConfigurationIs503 separates "no such route" from "cannot tell -// yet": an API error reading the GitProvider or its token must not answer 404, which a host may treat -// as a dead hook. +// yet": an API error reading the GitReceiver, its token or the providers must not answer 404, which +// a host may treat as a dead hook, nor a 202 that drops the delivery. func TestReceiverHandler_UnreadableConfigurationIs503(t *testing.T) { failing := errors.New("apiserver unavailable") - for name, funcs := range map[string]*interceptor.Funcs{ - "provider lookup fails": { - List: func(_ context.Context, _ client.WithWatch, _ client.ObjectList, _ ...client.ListOption) error { - return failing + failListOf := func(match func(client.ObjectList) bool) *interceptor.Funcs { + return &interceptor.Funcs{ + List: func(ctx context.Context, c client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if match(list) { + return failing + } + return c.List(ctx, list, opts...) + }, + } + } + failGetOf := func(match func(client.Object) bool) *interceptor.Funcs { + return &interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, + opts ...client.GetOption) error { + if match(obj) { + return failing + } + return c.Get(ctx, key, obj, opts...) }, + } + } + for name, tc := range map[string]struct { + funcs *interceptor.Funcs + body string + }{ + "receiver lookup fails": { + funcs: failListOf(func(l client.ObjectList) bool { _, ok := l.(*v1alpha3.GitReceiverList); return ok }), + body: hostPushBody("refs/heads/main", platformRepo), }, "token read fails": { - Get: func(_ context.Context, _ client.WithWatch, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { - return failing - }, + funcs: failGetOf(func(o client.Object) bool { _, ok := o.(*corev1.Secret); return ok }), + body: hostPushBody("refs/heads/main", platformRepo), + }, + "provider lookup fails": { + funcs: failListOf(func(l client.ObjectList) bool { _, ok := l.(*v1alpha3.GitProviderList); return ok }), + body: hostPushBody("refs/heads/main", platformRepo), }, } { t.Run(name, func(t *testing.T) { notifier := &fakePushNotifier{} - h := newReceiverTestHandler(t, notifier, 0, funcs) + h := newReceiverTestHandler(t, notifier, 0, tc.funcs) - body := hostPushBody("refs/heads/main") - req := httptest.NewRequest(http.MethodPost, route("gh"), strings.NewReader(body)) - for k, v := range githubHeaders("push", body, receiverTestToken) { + req := httptest.NewRequest(http.MethodPost, route("gh"), strings.NewReader(tc.body)) + for k, v := range githubHeaders("push", tc.body, receiverTestToken) { req.Header.Set(k, v) } rec := httptest.NewRecorder() h.ServeHTTP(rec, req) assert.Equal(t, http.StatusServiceUnavailable, rec.Code) - assert.Empty(t, notifier.calls) }) } } @@ -511,13 +622,13 @@ func TestReceiverHandler_UnreadableConfigurationIs503(t *testing.T) { // TestReceiverHandler_TooManyInFlightIs429 pins the in-flight cap: a request over it is answered // before anything is read, and a request after the others finished is served again. func TestReceiverHandler_TooManyInFlightIs429(t *testing.T) { - notifier := &fakePushNotifier{workers: map[string]bool{"main": true}} + notifier := &fakePushNotifier{workers: testWorkers()} h := newReceiverTestHandler(t, notifier, 0, nil) for range DefaultReceiverMaxInFlight { h.inFlight <- struct{}{} } - body := hostPushBody("refs/heads/main") + body := hostPushBody("refs/heads/main", platformRepo) post := func() *httptest.ResponseRecorder { req := httptest.NewRequest(http.MethodPost, route("gh"), strings.NewReader(body)) for k, v := range githubHeaders("push", body, receiverTestToken) { @@ -537,13 +648,13 @@ func TestReceiverHandler_TooManyInFlightIs429(t *testing.T) { <-h.inFlight } assert.Equal(t, http.StatusAccepted, post().Code) - assert.Len(t, notifier.calls, 1) + assert.Len(t, notifier.calls, 2, "both providers on the repository are asked") } -// TestReceiverPath_DoesNotNameTheProviderAndNeverCollides pins what the path is for: it reveals -// neither namespace nor name, it moves with the token, and two providers sharing a token whose +// TestReceiverPath_DoesNotNameTheReceiverAndNeverCollides pins what the path is for: it reveals +// neither namespace nor name, it moves with the token, and two receivers sharing a token whose // names concatenate alike (Flux's sha256(token+name+namespace) would collide on these) get two paths. -func TestReceiverPath_DoesNotNameTheProviderAndNeverCollides(t *testing.T) { +func TestReceiverPath_DoesNotNameTheReceiverAndNeverCollides(t *testing.T) { token := []byte(receiverTestToken) path := v1alpha3.ReceiverPath(token, "team-a", "platform") assert.True(t, strings.HasPrefix(path, ReceiverPathPrefix)) @@ -586,3 +697,32 @@ func TestVerifyDelivery_UnknownTypeFailsClosed(t *testing.T) { header.Set(headerGenericHMAC, "sha256="+hmacHex(string(body), receiverTestToken)) assert.False(t, verifyDelivery("bitbucket", header, body, []byte(receiverTestToken))) } + +// TestReceiverIndex indexes only a published path. +func TestReceiverIndex(t *testing.T) { + assert.Nil(t, IndexGitReceiverWebhookPath(&v1alpha3.GitProvider{}), "only GitReceivers carry a route") + assert.Nil(t, IndexGitReceiverWebhookPath(&v1alpha3.GitReceiver{}), "no published path, no route") + assert.Equal(t, []string{route("gh")}, IndexGitReceiverWebhookPath( + receiverTestReceiver("gh", v1alpha3.ReceiverTypeGitHub, "receiver-token"))) +} + +// TestParseDelivery_ReadsEveryRepositorySpelling collects each URL a host fills, so a provider +// spelling its repository any of those ways is the one the delivery is for, and none that names no +// clone URL. +func TestParseDelivery_ReadsEveryRepositorySpelling(t *testing.T) { + body := `{"ref":"refs/heads/main","repository":{` + + `"clone_url":"https://github.com/acme/platform.git","ssh_url":"git@github.com:acme/platform.git",` + + `"git_url":"git://github.com/acme/platform.git","html_url":"https://github.com/acme/platform",` + + `"url":"https://api.example/repos/acme/platform","homepage":"https://acme.example"},` + + `"project":{"git_ssh_url":"git@gitlab.example:acme/platform.git"}}` + delivery, err := parseDelivery(v1alpha3.ReceiverTypeGitHub, []byte(body)) + require.NoError(t, err) + assert.Equal(t, "main", delivery.branch) + assert.Equal(t, map[string]struct{}{ + "github.com/acme/platform": {}, + "gitlab.example/acme/platform": {}, + }, delivery.repositories) + assert.True(t, delivery.names("ssh://git@github.com:22/Acme/Platform")) + assert.False(t, delivery.names(""), "an empty URL names no repository") + assert.False(t, delivery.names("https://api.example/repos/acme/platform"), "an API URL is not read") +} diff --git a/test/e2e/Taskfile-e2e.yml b/test/e2e/Taskfile-e2e.yml index f156957d..a9119a38 100644 --- a/test/e2e/Taskfile-e2e.yml +++ b/test/e2e/Taskfile-e2e.yml @@ -1327,7 +1327,7 @@ tasks: CTX="{{.CTX}}" KUBECTL="{{.KUBECTL}}" bash hack/e2e/wait-crds-established.sh 60 \ crd/kubeproviders.configbutler.ai crd/clusterwatchrules.configbutler.ai \ crd/commitrequests.configbutler.ai crd/gitproviders.configbutler.ai \ - crd/gitbranches.configbutler.ai \ + crd/gitbranches.configbutler.ai crd/gitreceivers.configbutler.ai \ crd/gittargets.configbutler.ai crd/watchrules.configbutler.ai \ crd/fluxtriggers.configbutler.ai crd/argocdtriggers.configbutler.ai - touch "{{.CS}}/crds.installed" diff --git a/test/e2e/argocd_bi_directional_e2e_test.go b/test/e2e/argocd_bi_directional_e2e_test.go index b08bcebc..39391098 100644 --- a/test/e2e/argocd_bi_directional_e2e_test.go +++ b/test/e2e/argocd_bi_directional_e2e_test.go @@ -158,6 +158,7 @@ var _ = Describe("Bi Directional (Argo CD)", Label("bi-directional", "argocd"), cleanupWatchRule(run.watchRuleName, testNs) cleanupGitTarget(run.gitTargetName, testNs) cleanupNamespacedResource(testNs, "gitprovider", run.gitProviderName) + cleanupNamespacedResource(testNs, "gitreceiver", run.receiverSecretName) cleanupNamespacedResource(testNs, "secret", run.receiverSecretName) cleanupClusterResource("crd", iceCreamCRDName(crdGroupArgoBiDirectional)) cleanupNamespace(testNs) @@ -508,7 +509,7 @@ func (r argoBiDirectionalRun) startReverserPipeline() int64 { createGitProviderWithURLInNamespace( r.gitProviderName, r.testNs, argoBiDirectionalRepo.GitSecretHTTP, r.repoURL) verifyResourceStatus("gitprovider", r.gitProviderName, r.testNs, "True", "Succeeded", "") - hookID := registerOperatorReceiver(r.testNs, r.gitProviderName, r.receiverSecretName, r.repoName) + hookID := registerOperatorReceiver(r.testNs, r.receiverSecretName, r.repoName) createGitTarget(r.gitTargetName, r.testNs, r.gitProviderName, r.livePath, "main") createArgoCDTrigger(r.testNs, r.appName, r.argoNs, r.appName) diff --git a/test/e2e/bi_directional_timing_e2e_test.go b/test/e2e/bi_directional_timing_e2e_test.go index 29fdddd5..1e2d7224 100644 --- a/test/e2e/bi_directional_timing_e2e_test.go +++ b/test/e2e/bi_directional_timing_e2e_test.go @@ -126,7 +126,7 @@ func (p *timingPipeline) start() { createGitProviderWithURLInNamespace(p.providerName, p.testNs, p.repo.GitSecretHTTP, p.repo.RepoURLHTTP) verifyResourceStatus("gitprovider", p.providerName, p.testNs, "True", "Succeeded", "") - p.hookID = registerOperatorReceiver(p.testNs, p.providerName, p.receiverName, p.repo.RepoName) + p.hookID = registerOperatorReceiver(p.testNs, p.receiverName, p.repo.RepoName) createGitTarget(p.targetName, p.testNs, p.providerName, p.gitPath, "main") _, err = kubectlRunWithStdin(p.testNs, fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 @@ -168,6 +168,7 @@ func (p *timingPipeline) cleanup() { cleanupWatchRule(p.ruleName, p.testNs) cleanupGitTarget(p.targetName, p.testNs) cleanupNamespacedResource(p.testNs, "gitprovider", p.providerName) + cleanupNamespacedResource(p.testNs, "gitreceiver", p.receiverName) cleanupNamespacedResource(p.testNs, "secret", p.receiverName) } diff --git a/test/e2e/flux_applier_e2e_test.go b/test/e2e/flux_applier_e2e_test.go index db615bd9..a5c9cea6 100644 --- a/test/e2e/flux_applier_e2e_test.go +++ b/test/e2e/flux_applier_e2e_test.go @@ -69,6 +69,7 @@ var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Order cleanupGitTarget(destName, testNs) cleanupNamespacedResource(testNs, fluxTriggerResource, fluxName) deleteGitProvider(testNs, providerName) + cleanupNamespacedResource(testNs, "gitreceiver", receiverSecretName) cleanupNamespacedResource(testNs, "secret", receiverSecretName) cleanupNamespacedResource("flux-system", "kustomization", fluxName) cleanupNamespacedResource("flux-system", "gitrepository", fluxName) @@ -207,8 +208,8 @@ var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Order // event, signed, to the operator's receiver Service, and nothing else happens on the branch: no // API edit surfaces the push, and the refresher's tick is minutes away. It("applies a Git-side push the Git host reports, with nothing else to surface it", func() { - By("configuring the GitProvider's receiver, and registering it as a Gitea webhook") - receiverHookID = registerOperatorReceiver(testNs, providerName, receiverSecretName, repo.RepoName) + By("creating a GitReceiver, and registering it as a Gitea webhook") + receiverHookID = registerOperatorReceiver(testNs, receiverSecretName, repo.RepoName) Eventually(func(g Gomega) { g.Expect(appliedColor(g, appliedNs)).To(Equal("orange")) @@ -227,6 +228,14 @@ var _ = Describe("Manager Flux Applier", Label("manager", "flux-applier"), Order Expect(strings.Count(gitTargetEventMessages(Default, testNs, destName, "ApplyRequested"), "(ForeignCommit)")).To(BeNumerically(">", foreignBefore), "the apply must come from a ForeignCommit request, not from Flux's own schedule") + + By("the GitBranch records that the delivery reached it") + Eventually(func(g Gomega) { + out, err := kubectlRunInNamespace(testNs, "get", "gitbranch", gitBranchName(providerName, "main"), + "-o", "jsonpath={.status.lastPushNoticeAt}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(strings.TrimSpace(out)).NotTo(BeEmpty()) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) }) // The suspended lifecycle's two promises beyond an ordinary apply: a failed apply is suspended @@ -414,24 +423,40 @@ func gitTargetEventMessages(g Gomega, testNs, gitTarget, reason string) string { return out } -// registerOperatorReceiver gives a GitProvider a gitea receiver whose token is a new Secret, and -// registers the path it serves as a push webhook on the Gitea repository, pointed at the operator's -// receiver Service. It returns the hook's id, for deleteRepoHook. -func registerOperatorReceiver(testNs, providerName, secretName, repoName string) int64 { +// registerOperatorReceiver creates a gitea GitReceiver named name in testNs, whose token is a new +// Secret of that name too, and registers its path as a Gitea webhook on the repository, pointed at +// the operator's receiver Service. The receiver routes by URL, so it wakes the repository's +// GitProviders without either naming the other. It waits until the GitReceiver is Ready, so a push +// from here on is delivered. It returns the hook's id, for deleteRepoHook; the caller deletes the +// GitReceiver and the Secret, both named name. +func registerOperatorReceiver(testNs, name, repoName string) int64 { GinkgoHelper() token := randomToken() - _, err := kubectlRunInNamespace(testNs, "create", "secret", "generic", secretName, "--from-literal=token="+token) + _, err := kubectlRunInNamespace(testNs, "create", "secret", "generic", name, "--from-literal=token="+token) Expect(err).NotTo(HaveOccurred(), "failed to create the receiver Secret") - _, err = kubectlRunInNamespace(testNs, "patch", "gitprovider", providerName, "--type=merge", "-p", - fmt.Sprintf(`{"spec":{"receiver":{"type":"gitea","secretRef":{"name":%q}}}}`, secretName)) - Expect(err).NotTo(HaveOccurred(), "failed to set spec.receiver") + receiver := fmt.Sprintf(`apiVersion: configbutler.ai/v1alpha3 +kind: GitReceiver +metadata: + name: %s + namespace: %s +spec: + type: gitea + secretRef: + name: %s +`, name, testNs, name) + _, err = kubectlRunWithStdin(testNs, receiver, "apply", "-f", "-") + Expect(err).NotTo(HaveOccurred(), "failed to create the GitReceiver") var webhookPath string Eventually(func(g Gomega) { - out, err := kubectlRunInNamespace(testNs, "get", "gitprovider", providerName, + out, err := kubectlRunInNamespace(testNs, "get", "gitreceiver", name, "-o", "jsonpath={.status.webhookPath}") g.Expect(err).NotTo(HaveOccurred()) webhookPath = strings.TrimSpace(out) g.Expect(webhookPath).NotTo(BeEmpty()) + ready, err := kubectlRunInNamespace(testNs, "get", "gitreceiver", name, + "-o", `jsonpath={.status.conditions[?(@.type=="Ready")].status}`) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(strings.TrimSpace(ready)).To(Equal("True"), "the GitReceiver serves deliveries") }, time.Minute, 2*time.Second).Should(Succeed()) gitea := giteaTestInstance() ctx, cancel := gitea.Context()