Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions api/v1alpha1/config_validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package v1alpha1

import (
"fmt"
"strings"
)

const webhookNotificationSourceType = "Webhook"

// Validate checks cross-field constraints on ConfigSpec that cannot be expressed on individual fields.
func (s *ConfigSpec) Validate() error {
if err := validateUniqueWebhookPathSuffixes(s.NotificationSources); err != nil {
return err
}
return nil
}

func validateUniqueWebhookPathSuffixes(sources []NotificationSource) error {
seen := map[string]int{}
var duplicates []string

for i, source := range sources {
if source.Type != webhookNotificationSourceType {
continue
}

suffix := ""
if source.Webhook != nil {
suffix = strings.TrimSpace(source.Webhook.PathSuffix)
}

if firstIndex, exists := seen[suffix]; exists {
if suffix == "" {
duplicates = append(duplicates, fmt.Sprintf(
"notificationSources[%d] and notificationSources[%d] both omit webhook.pathSuffix; at most one webhook without pathSuffix is allowed",
firstIndex, i,
))
} else {
duplicates = append(duplicates, fmt.Sprintf(
"notificationSources[%d].webhook.pathSuffix %q duplicates notificationSources[%d].webhook.pathSuffix",
i, suffix, firstIndex,
))
}
continue
}
seen[suffix] = i
}

if len(duplicates) == 0 {
return nil
}
return fmt.Errorf("%s", strings.Join(duplicates, "; "))
}
131 changes: 131 additions & 0 deletions api/v1alpha1/config_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package v1alpha1

import "testing"

func TestConfigSpecValidate_UniqueWebhookPathSuffixes(t *testing.T) {
t.Parallel()

tests := []struct {
name string
spec ConfigSpec
wantErr bool
}{
{
name: "single webhook without pathSuffix",
spec: ConfigSpec{
NotificationSources: []NotificationSource{{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{},
}},
},
},
{
name: "multiple webhooks with distinct pathSuffixes",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-b"},
},
},
},
},
{
name: "one empty and one distinct pathSuffix",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{},
},
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
},
},
},
{
name: "multiple webhooks without pathSuffix",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{},
},
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{},
},
},
},
wantErr: true,
},
{
name: "multiple webhooks with nil webhook block",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{Type: webhookNotificationSourceType},
{Type: webhookNotificationSourceType},
},
},
wantErr: true,
},
{
name: "duplicate pathSuffix",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
},
},
wantErr: true,
},
{
name: "duplicate pathSuffix ignores non-webhook sources",
spec: ConfigSpec{
NotificationSources: []NotificationSource{
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
{
Type: "TCPSocket",
TCPSocket: &TCPSocketConfig{
Host: "127.0.0.1",
Port: 8000,
},
},
{
Type: webhookNotificationSourceType,
Webhook: &WebhookConfig{PathSuffix: "vendor-a"},
},
},
},
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := tt.spec.Validate()
if tt.wantErr && err == nil {
t.Fatal("expected validation error")
}
if !tt.wantErr && err != nil {
t.Fatalf("unexpected validation error: %v", err)
}
})
}
}
19 changes: 9 additions & 10 deletions api/v1alpha1/source_webhook_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,20 @@ package v1alpha1

// WebhookConfig contains configuration for Webhook notifications.
type WebhookConfig struct {
// Path that the webhook will receive the notifications.
// If not present `/webhook` will be used. The path always expects a POST and this is not configurable
// +optional
Path string `json:"path"`

// Address is the address where the webhook will be served in your infrastructure.
// If not present, defaults to `:8090`
// +optional
Address string `json:"address"`

Comment thread
pepordev marked this conversation as resolved.
// SecretIdentifierOnPayload is the key that the reloader will look for in the payload.
// The value of this key should be the same name as in the external secret. It will default to `0.data.ObjectName` if not set
// +optional
SecretIdentifierOnPayload string `json:"identifierPathOnPayload,omitempty"`

// PathSuffix is an optional URL path segment for this webhook notification source.
// When set, the webhook is reachable at POST /webhook/<Config.metadata.name>/<pathSuffix>.
// Use distinct suffixes when a Config defines multiple webhook notification sources.
// At most one webhook source may omit pathSuffix; duplicate values (including multiple empty) are rejected.
// +optional
// +kubebuilder:validation:MaxLength=63
// +kubebuilder:validation:Pattern=`^[a-zA-Z0-9]([a-zA-Z0-9_-]*[a-zA-Z0-9])?$`
PathSuffix string `json:"pathSuffix,omitempty"`

// Auth is the authentication method for the webhook
// +optional
Auth *WebhookAuth `json:"webhookAuth,omitempty"`
Expand Down
24 changes: 12 additions & 12 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

externalsecrets "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
pushsecrets "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"

"github.com/external-secrets/reloader/api/v1alpha1"
"github.com/external-secrets/reloader/internal/controller"
"github.com/external-secrets/reloader/internal/listener/webhook"
// +kubebuilder:scaffold:imports
)

Expand Down Expand Up @@ -78,8 +78,8 @@ func main() {
flag.StringVar(
&webhookAddr,
"webhook-bind-address",
":8082",
"The address the webhook listener binds to. Defaults to :8082",
":8090",
"The address the webhook listener binds to. Defaults to :8090",
)

opts := zap.Options{
Expand All @@ -99,26 +99,21 @@ func main() {
tlsOpts = append(tlsOpts, disableHTTP2)
}

webhookServer := webhook.NewServer(webhook.Options{
TLSOpts: tlsOpts,
})

metricsServerOptions := metricsserver.Options{
BindAddress: metricsAddr,
SecureServing: secureMetrics,

TLSOpts: tlsOpts,
TLSOpts: tlsOpts,
}

if secureMetrics {

metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization
}

notificationWebhook := webhook.NewWebhookServer(webhookAddr, ctrl.Log.WithName("notification"))

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: scheme,
Metrics: metricsServerOptions,
WebhookServer: webhookServer,
HealthProbeBindAddress: probeAddr,
LeaderElection: enableLeaderElection,
LeaderElectionID: "0cd7d2f7.externalsecrets.com",
Expand All @@ -128,14 +123,19 @@ func main() {
os.Exit(1)
}

if err := mgr.Add(notificationWebhook); err != nil {
setupLog.Error(err, "unable to add notification webhook server")
os.Exit(1)
}

if err = (controller.NewReloaderReconciler(
mgr.GetClient(),
mgr.GetScheme(),
notificationWebhook,
)).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Reloader")
os.Exit(1)
}

// +kubebuilder:scaffold:builder

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1184,20 +1184,19 @@ spec:
webhook:
description: Webhook configuration (required if Type is Webhook).
properties:
address:
description: |-
Address is the address where the webhook will be served in your infrastructure.
If not present, defaults to `:8090`
type: string
identifierPathOnPayload:
description: |-
SecretIdentifierOnPayload is the key that the reloader will look for in the payload.
The value of this key should be the same name as in the external secret. It will default to `0.data.ObjectName` if not set
type: string
path:
pathSuffix:
description: |-
Path that the webhook will receive the notifications.
If not present `/webhook` will be used. The path always expects a POST and this is not configurable
PathSuffix is an optional URL path segment for this webhook notification source.
When set, the webhook is reachable at POST /webhook/<Config.metadata.name>/<pathSuffix>.
Use distinct suffixes when a Config defines multiple webhook notification sources.
At most one webhook source may omit pathSuffix; duplicate values (including multiple empty) are rejected.
maxLength: 63
pattern: ^[a-zA-Z0-9]([a-zA-Z0-9_-]*[a-zA-Z0-9])?$
type: string
retryPolicy:
description: |-
Expand Down
12 changes: 9 additions & 3 deletions deploy/charts/reloader/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ spec:
args:
- --health-probe-bind-address=:8081
- --metrics-bind-address=:{{ .Values.metrics.listen.port }}
- --metrics-secure={{ .Values.metrics.secure }}
{{- if .Values.service.webhook.enabled }}
- --webhook-bind-address=:{{ .Values.service.webhook.listenPort }}
{{- end }}
{{- if .Values.leaderElect }}
- --leader-elect
{{- end }}
Expand All @@ -66,15 +70,17 @@ spec:
{{- toYaml . | nindent 12 }}
{{- end }}
ports:
- name: http
containerPort: {{ .Values.service.port }}
protocol: TCP
- name: healthz
containerPort: 8081
protocol: TCP
- name: metrics
containerPort: {{ .Values.metrics.listen.port }}
protocol: TCP
{{- if .Values.service.webhook.enabled }}
- name: webhook
containerPort: {{ .Values.service.webhook.listenPort }}
protocol: TCP
{{- end }}
livenessProbe:
{{- toYaml .Values.livenessProbe | nindent 12 }}
readinessProbe:
Expand Down
11 changes: 7 additions & 4 deletions deploy/charts/reloader/templates/ingress.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
{{- if .Values.ingress.enabled -}}
{{- if not .Values.service.webhook.enabled }}
{{- fail "ingress.enabled requires service.webhook.enabled (Ingress routes to the webhook Service)" }}
{{- end }}
{{- $fullName := include "reloader.fullname" . -}}
{{- $svcPort := .Values.service.port -}}
{{- $svcPort := .Values.service.webhook.port -}}
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: {{ $fullName }}
name: {{ $fullName }}-webhook
namespace: {{ include "reloader.namespace" . }}
labels:
{{- include "reloader.labels" . | nindent 4 }}
Expand Down Expand Up @@ -33,10 +36,10 @@ spec:
paths:
{{- range .paths }}
- path: {{ .path }}
pathType: {{ .pathType | default "ImplementationSpecific" }}
pathType: {{ .pathType | default "Prefix" }}
backend:
service:
name: {{ $fullName }}
name: {{ $fullName }}-webhook
port:
number: {{ $svcPort }}
{{- end }}
Expand Down
5 changes: 5 additions & 0 deletions deploy/charts/reloader/templates/podmonitor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,10 @@ spec:
interval: {{ .Values.podMonitor.interval }}
scrapeTimeout: {{ .Values.podMonitor.scrapeTimeout }}
honorLabels: {{ .Values.podMonitor.honorLabels }}
{{- if .Values.metrics.secure }}
scheme: https
tlsConfig:
insecureSkipVerify: true
{{- end }}
{{- end }}
{{- end }}
Loading
Loading