Skip to content

Commit a81b9d0

Browse files
efiacorefiacor
authored
Repo webhook move (#1129)
* Move repo val webhook Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Unique name for each webhook Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Address Copilot review comments on repository webhook - Use GetAPIReader() instead of GetClient() for strong consistency during admission validation (prevents cache-induced race conditions) - Scope repository list query to namespace level to reduce admission latency at scale - Test UPDATE operation semantics explicitly for the 'updating self' test case - Fix misleading documentation comment to link to actual deployment config All unit tests pass. No lint issues. Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Increase test coverage Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Block repo spec.git secret updates Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Move repo secret validation to CEL Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Revert git secret immutability Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Update after rebase Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Revert repo list on namespace and add indexers - Remove namespace-only filter from webhook conflict detection - Add field indexes on spec.git.repo and spec.git.branch for optimization - Add comprehensive test coverage for indexing functions - Condense and improve webhook query comments Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Fall back to full List when no indexers Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Add matchConds to webhook and extra testing Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech> --------- Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
1 parent c900891 commit a81b9d0

17 files changed

Lines changed: 2707 additions & 1467 deletions

File tree

‎.vscode/launch.json‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232
],
3333
"cwd": "${workspaceFolder}",
3434
"env": {
35-
"CERT_STORAGE_DIR": "${workspaceFolder}/.build/pki/tmp",
36-
"WEBHOOK_HOST": "localhost",
3735
"GOOGLE_API_GO_EXPERIMENTAL_DISABLE_NEW_AUTH_LIB": "true",
3836
"OTEL_TRACES_EXPORTER": "none",
3937
"OTEL_METRICS_EXPORTER": "none"
@@ -56,8 +54,6 @@
5654
],
5755
"cwd": "${workspaceFolder}",
5856
"env": {
59-
"CERT_STORAGE_DIR": "${workspaceFolder}/.build/pki/tmp",
60-
"WEBHOOK_HOST": "localhost",
6157
"GOOGLE_API_GO_EXPERIMENTAL_DISABLE_NEW_AUTH_LIB": "true",
6258
"DB_DRIVER": "pgx",
6359
"DB_HOST": "${env:DB_HOST}",

‎controllers/packagerevisions/config/webhook/manifests.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
apiVersion: admissionregistration.k8s.io/v1
1616
kind: ValidatingWebhookConfiguration
1717
metadata:
18-
name: validating-webhook-configuration
18+
name: packagerevision-validating-webhook-configuration
1919
webhooks:
2020
- admissionReviewVersions:
2121
- v1

‎controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ import (
3434

3535
//go:generate go run sigs.k8s.io/controller-tools/cmd/controller-gen@v0.21.0 rbac:headerFile=../../../../../scripts/boilerplate.yaml.txt,roleName=porch-controllers-packagerevisions,year=$YEAR_GEN webhook:headerFile=../../../../../scripts/boilerplate.yaml.txt,year=$YEAR_GEN paths="." output:rbac:artifacts:config=../../../config/rbac output:webhook:artifacts:config=../../../config/webhook
3636

37+
//+kubebuilder:webhookconfiguration:mutating=false,name=packagerevision-validating-webhook-configuration
3738
//+kubebuilder:webhook:path=/validate-porch-kpt-dev-v1alpha2-packagerevision,mutating=false,failurePolicy=fail,groups=porch.kpt.dev,resources=packagerevisions,verbs=create;update;delete,versions=v1alpha2,name=packagerevision-validator.porch.kpt.dev,admissionReviewVersions=v1,sideEffects=None,serviceName=porch-controllers,serviceNamespace=porch-system,servicePort=9443,timeoutSeconds=30
38-
3939
//+kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions,verbs=get;list;watch;update;patch
4040
//+kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions/status,verbs=get;update;patch
4141
//+kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions/finalizers,verbs=update

‎controllers/repositories/config/webhook/manifests.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Copyright 2026 The kpt Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
---
15+
apiVersion: admissionregistration.k8s.io/v1
16+
kind: ValidatingWebhookConfiguration
17+
metadata:
18+
name: repository-validating-webhook-configuration
19+
webhooks:
20+
- admissionReviewVersions:
21+
- v1
22+
clientConfig:
23+
service:
24+
name: porch-controllers
25+
namespace: porch-system
26+
path: /validate-repository
27+
port: 9443
28+
failurePolicy: Fail
29+
matchConditions:
30+
- expression: request.operation == 'CREATE' || request.operation == 'DELETE' || object.metadata.generation != oldObject.metadata.generation
31+
name: skip-non-spec-updates
32+
name: repository-validator.porch.kpt.dev
33+
rules:
34+
- apiGroups:
35+
- config.porch.kpt.dev
36+
apiVersions:
37+
- v1alpha1
38+
operations:
39+
- CREATE
40+
- UPDATE
41+
- DELETE
42+
resources:
43+
- repositories
44+
sideEffects: None
45+
timeoutSeconds: 30

‎controllers/repositories/pkg/controllers/repository/config.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,17 @@
1515
package repository
1616

1717
import (
18+
"context"
1819
"flag"
20+
"fmt"
1921
"time"
2022

23+
configapi "github.com/kptdev/porch/api/porchconfig/v1alpha1"
24+
"github.com/kptdev/porch/controllers/repositories/pkg/webhooks"
2125
cachetypes "github.com/kptdev/porch/pkg/cache/types"
26+
ctrl "sigs.k8s.io/controller-runtime"
27+
"sigs.k8s.io/controller-runtime/pkg/client"
28+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2229
)
2330

2431
const (
@@ -87,6 +94,51 @@ func (r *RepositoryReconciler) validateConfig() {
8794
}
8895
}
8996

97+
// Init wires runtime dependencies that require the manager.
98+
// It registers the Repository validating webhook on the shared webhook server
99+
// and sets up field indexes for efficient conflict detection queries.
100+
func (r *RepositoryReconciler) Init(mgr ctrl.Manager) error {
101+
log := ctrl.Log.WithName(r.Name())
102+
103+
// Set up field indexes for Repository conflict detection
104+
// These indexes allow efficient querying by git location without listing all repositories
105+
ctx := context.Background()
106+
if err := mgr.GetFieldIndexer().IndexField(ctx, &configapi.Repository{}, "spec.git.repo", func(o client.Object) []string {
107+
repository := o.(*configapi.Repository)
108+
if repository.Spec.Git == nil || repository.Spec.Git.Repo == "" {
109+
return nil
110+
}
111+
return []string{repository.Spec.Git.Repo}
112+
}); err != nil {
113+
return fmt.Errorf("error indexing Repository by git.repo: %w", err)
114+
}
115+
116+
if err := mgr.GetFieldIndexer().IndexField(ctx, &configapi.Repository{}, "spec.git.branch", func(o client.Object) []string {
117+
repository := o.(*configapi.Repository)
118+
if repository.Spec.Git == nil || repository.Spec.Git.Branch == "" {
119+
return nil
120+
}
121+
return []string{repository.Spec.Git.Branch}
122+
}); err != nil {
123+
return fmt.Errorf("error indexing Repository by git.branch: %w", err)
124+
}
125+
126+
// Register Repository validating webhook.
127+
// The validator implements admission.Handler interface via its Handle method.
128+
// Use GetAPIReader() for strong consistency during admission validation.
129+
// See: deployments/porch/3-porch-controllers.yaml
130+
validator := webhooks.NewRepositoryValidator(mgr.GetAPIReader())
131+
132+
mgr.GetWebhookServer().Register(
133+
"/validate-repository",
134+
&admission.Webhook{
135+
Handler: validator,
136+
})
137+
log.Info("Repository validating webhook registered")
138+
139+
return nil
140+
}
141+
90142
// LogConfig logs the controller configuration
91143
func (r *RepositoryReconciler) LogConfig(log interface {
92144
Info(msg string, keysAndValues ...any)

0 commit comments

Comments
 (0)