Skip to content

Commit f33a83f

Browse files
Wei WengWei Weng
Wei Weng
authored and
Wei Weng
committed
make some configurable and add validation and document
Signed-off-by: Wei Weng <Wei.Weng@microsoft.com>
1 parent 73e8354 commit f33a83f

File tree

10 files changed

+117
-23
lines changed

10 files changed

+117
-23
lines changed

‎charts/hub-agent/README.md‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ helm install cert-manager jetstack/cert-manager \
2424
--set crds.enabled=true
2525

2626
# Then install hub-agent with cert-manager enabled
27-
helm install hub-agent ./charts/hub-agent --set useCertManager=true
27+
helm install hub-agent ./charts/hub-agent --set useCertManager=true --set enableWorkload=true --set enableWebhook=true
2828
```
2929

3030
This configures cert-manager to manage webhook certificates.
@@ -58,7 +58,9 @@ _See [helm install](https://helm.sh/docs/helm/helm_install/) for command documen
5858
| `webhookServiceName` | Webhook service name | `fleetwebhook` |
5959
| `enableGuardRail` | Enable guard rail webhook configurations | `true` |
6060
| `webhookClientConnectionType` | Connection type for webhook client (service or url) | `service` |
61-
| `useCertManager` | Use cert-manager for webhook certificate management | `false` |
61+
| `useCertManager` | Use cert-manager for webhook certificate management (requires `enableWebhook=true` and `enableWorkload=true`) | `false` |
62+
| `webhookCertDir` | Directory where webhook certificates are stored/mounted | `/tmp/k8s-webhook-server/serving-certs` |
63+
| `webhookCertSecretName` | Name of the Secret containing webhook certificates | `fleet-webhook-server-cert` |
6264
| `enableV1Beta1APIs` | Watch for v1beta1 APIs | `true` |
6365
| `hubAPIQPS` | QPS for fleet-apiserver (not including events/node heartbeat) | `250` |
6466
| `hubAPIBurst` | Burst for fleet-apiserver (not including events/node heartbeat) | `1000` |
@@ -85,6 +87,8 @@ By default, the hub-agent generates certificates automatically at startup. This
8587

8688
When `useCertManager=true`, certificates are managed by cert-manager. This mode:
8789
- Requires cert-manager to be installed as a prerequisite
90+
- Requires `enableWorkload=true` to allow cert-manager pods to run in the hub cluster (without this, pod creation would be blocked by the webhook)
91+
- Requires `enableWebhook=true` because cert-manager is only used for webhook certificate management
8892
- Handles certificate rotation automatically (90-day certificates)
8993
- Follows industry-standard certificate management practices
9094
- Suitable for production environments
@@ -101,5 +105,26 @@ helm install cert-manager jetstack/cert-manager \
101105
--set crds.enabled=true
102106

103107
# Then install hub-agent with cert-manager enabled
104-
helm install hub-agent ./charts/hub-agent --set useCertManager=true
108+
helm install hub-agent ./charts/hub-agent --set useCertManager=true --set enableWorkload=true --set enableWebhook=true
109+
```
110+
111+
### Certificate Directory Configuration
112+
113+
The `webhookCertDir` parameter allows you to customize where webhook certificates are stored:
114+
- Default: `/tmp/k8s-webhook-server/serving-certs`
115+
- Must match the volumeMount path when using cert-manager
116+
- Configurable via both Helm values and `--webhook-cert-dir` flag
117+
118+
The `webhookCertSecretName` parameter allows you to customize the Secret name for webhook certificates:
119+
- Default: `fleet-webhook-server-cert`
120+
- When using cert-manager, this must match the Certificate resource's secretName
121+
- Configurable via both Helm values and `--webhook-cert-secret-name` flag
122+
123+
Example with custom certificate directory and secret name:
124+
```console
125+
helm install hub-agent ./charts/hub-agent \
126+
--set useCertManager=true \
127+
--set enableWorkload=true \
128+
--set webhookCertDir=/custom/cert/path \
129+
--set webhookCertSecretName=my-webhook-cert
105130
```

‎charts/hub-agent/templates/certificate.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
apiVersion: cert-manager.io/v1
44
kind: Certificate
55
metadata:
6-
name: fleet-webhook-server-cert
6+
name: {{ .Values.webhookCertSecretName }}
77
namespace: {{ .Values.namespace }}
88
labels:
99
{{- include "hub-agent.labels" . | nindent 4 }}
1010
spec:
1111
# Secret name where cert-manager will store the certificate
12-
secretName: fleet-webhook-server-cert
12+
secretName: {{ .Values.webhookCertSecretName }}
1313

1414
# Certificate duration (90 days is cert-manager's default and recommended)
1515
duration: 2160h # 90 days

‎charts/hub-agent/templates/deployment.yaml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ spec:
3030
- --enable-guard-rail={{ .Values.enableGuardRail }}
3131
- --enable-workload={{ .Values.enableWorkload }}
3232
- --use-cert-manager={{ .Values.useCertManager }}
33+
- --webhook-cert-dir={{ .Values.webhookCertDir }}
34+
- --webhook-cert-secret-name={{ .Values.webhookCertSecretName }}
3335
- --whitelisted-users=system:serviceaccount:fleet-system:hub-agent-sa
3436
- --webhook-client-connection-type={{.Values.webhookClientConnectionType}}
3537
- --v={{ .Values.logVerbosity }}
@@ -81,14 +83,16 @@ spec:
8183
{{- if .Values.useCertManager }}
8284
volumeMounts:
8385
- name: webhook-cert
84-
mountPath: /tmp/k8s-webhook-server/serving-certs
86+
mountPath: {{ .Values.webhookCertDir }}
8587
readOnly: true
8688
{{- end }}
8789
{{- if .Values.useCertManager }}
8890
volumes:
8991
- name: webhook-cert
9092
secret:
91-
secretName: fleet-webhook-server-cert
93+
secretName: {{ .Values.webhookCertSecretName }}
94+
# defaultMode 0400 (read-only for owner) prevents unauthorized access to certificate files
95+
# and reduces attack surface by ensuring only the container process can read the certs
9296
defaultMode: 0400
9397
{{- end }}
9498
{{- with .Values.affinity }}

‎charts/hub-agent/values.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ enableWorkload: false
2121
# When enabled, cert-manager will be installed as a dependency
2222
# and a Certificate resource will be created
2323
useCertManager: false
24+
# webhookCertDir is the directory where webhook certificates are mounted
25+
# This is configurable via --webhook-cert-dir flag and must match the volumeMount path in deployment
26+
webhookCertDir: /tmp/k8s-webhook-server/serving-certs
27+
# webhookCertSecretName is the name of the Secret containing webhook certificates
28+
# When using cert-manager, this must match the Certificate resource's secretName
29+
webhookCertSecretName: fleet-webhook-server-cert
2430

2531
forceDeleteWaitTime: 15m0s
2632
clusterUnhealthyThreshold: 3m0s

‎cmd/hubagent/main.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ var (
6565
)
6666

6767
const (
68-
FleetWebhookCertDir = "/tmp/k8s-webhook-server/serving-certs"
69-
FleetWebhookPort = 9443
68+
FleetWebhookPort = 9443
7069
)
7170

7271
func init() {
@@ -121,7 +120,7 @@ func main() {
121120
},
122121
WebhookServer: ctrlwebhook.NewServer(ctrlwebhook.Options{
123122
Port: FleetWebhookPort,
124-
CertDir: FleetWebhookCertDir,
123+
CertDir: opts.WebhookCertDir,
125124
}),
126125
}
127126
if opts.EnablePprof {
@@ -159,7 +158,7 @@ func main() {
159158
if opts.EnableWebhook {
160159
whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",")
161160
if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers,
162-
opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled, opts.UseCertManager); err != nil {
161+
opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled, opts.UseCertManager, opts.WebhookCertDir, opts.WebhookCertSecretName); err != nil {
163162
klog.ErrorS(err, "unable to set up webhook")
164163
exitWithErrorFunc()
165164
}
@@ -202,9 +201,9 @@ func main() {
202201

203202
// SetupWebhook generates the webhook cert and then set up the webhook configurator.
204203
func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string,
205-
whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool, useCertManager bool) error {
206-
// Generate self-signed key and crt files in FleetWebhookCertDir for the webhook server to start.
207-
w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, FleetWebhookCertDir, enableGuardRail, denyModifyMemberClusterLabels, enableWorkload, useCertManager)
204+
whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool, useCertManager bool, webhookCertDir string, webhookCertSecretName string) error {
205+
// Generate self-signed key and crt files in webhookCertDir for the webhook server to start.
206+
w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, webhookCertDir, enableGuardRail, denyModifyMemberClusterLabels, enableWorkload, useCertManager, webhookCertSecretName)
208207
if err != nil {
209208
klog.ErrorS(err, "fail to generate WebhookConfig")
210209
return err

‎cmd/hubagent/options/options.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ type Options struct {
113113
// UseCertManager indicates whether to use cert-manager for webhook certificate management.
114114
// When enabled, webhook certificates are managed by cert-manager instead of self-signed generation.
115115
UseCertManager bool
116+
// WebhookCertDir is the directory where webhook certificates are stored/mounted.
117+
// This must match the mountPath in the Helm chart deployment when using cert-manager.
118+
WebhookCertDir string
119+
// WebhookCertSecretName is the name of the Secret containing webhook certificates.
120+
// When using cert-manager, this is the Secret name created by the Certificate resource.
121+
WebhookCertSecretName string
116122
// ResourceSnapshotCreationMinimumInterval is the minimum interval at which resource snapshots could be created.
117123
// Whether the resource snapshot is created or not depends on the both ResourceSnapshotCreationMinimumInterval and ResourceChangesCollectionDuration.
118124
ResourceSnapshotCreationMinimumInterval time.Duration
@@ -189,6 +195,8 @@ func (o *Options) AddFlags(flags *flag.FlagSet) {
189195
flags.BoolVar(&o.DenyModifyMemberClusterLabels, "deny-modify-member-cluster-labels", false, "If set, users not in the system:masters cannot modify member cluster labels.")
190196
flags.BoolVar(&o.EnableWorkload, "enable-workload", false, "If set, workloads (pods and replicasets) can be created in the hub cluster. This disables the pod and replicaset validating webhooks.")
191197
flags.BoolVar(&o.UseCertManager, "use-cert-manager", false, "If set, cert-manager will be used for webhook certificate management instead of self-signed certificates.")
198+
flags.StringVar(&o.WebhookCertDir, "webhook-cert-dir", "/tmp/k8s-webhook-server/serving-certs", "The directory where webhook certificates are stored. Must match the volumeMount path in deployment when using cert-manager.")
199+
flags.StringVar(&o.WebhookCertSecretName, "webhook-cert-secret-name", "fleet-webhook-server-cert", "The name of the Secret containing webhook certificates. Must match the secretName in deployment and Certificate resource when using cert-manager.")
192200
flags.DurationVar(&o.ResourceSnapshotCreationMinimumInterval, "resource-snapshot-creation-minimum-interval", 30*time.Second, "The minimum interval at which resource snapshots could be created.")
193201
flags.DurationVar(&o.ResourceChangesCollectionDuration, "resource-changes-collection-duration", 15*time.Second,
194202
"The duration for collecting resource changes into one snapshot. The default is 15 seconds, which means that the controller will collect resource changes for 15 seconds before creating a resource snapshot.")

‎cmd/hubagent/options/validation.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ func (o *Options) Validate() field.ErrorList {
5252
errs = append(errs, field.Invalid(newPath.Child("WebhookServiceName"), o.WebhookServiceName, "Webhook service name is required when webhook is enabled"))
5353
}
5454

55+
if o.UseCertManager && !o.EnableWebhook {
56+
errs = append(errs, field.Invalid(newPath.Child("UseCertManager"), o.UseCertManager, "UseCertManager requires EnableWebhook to be true (cert-manager is only used for webhook certificate management)"))
57+
}
58+
59+
if o.UseCertManager && !o.EnableWorkload {
60+
errs = append(errs, field.Invalid(newPath.Child("UseCertManager"), o.UseCertManager, "UseCertManager requires EnableWorkload to be true (cert-manager pods need to run in the hub cluster)"))
61+
}
62+
5563
connectionType := o.WebhookClientConnectionType
5664
if _, err := parseWebhookClientConnectionString(connectionType); err != nil {
5765
errs = append(errs, field.Invalid(newPath.Child("WebhookClientConnectionType"), o.WebhookClientConnectionType, err.Error()))

‎cmd/hubagent/options/validation_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ import (
2727
"k8s.io/apimachinery/pkg/util/validation/field"
2828
)
2929

30+
const (
31+
testWebhookServiceName = "test-webhook"
32+
)
33+
3034
// a callback function to modify options
3135
type ModifyOptions func(option *Options)
3236

@@ -93,6 +97,42 @@ func TestValidateControllerManagerConfiguration(t *testing.T) {
9397
}),
9498
want: field.ErrorList{field.Invalid(newPath.Child("WebhookServiceName"), "", "Webhook service name is required when webhook is enabled")},
9599
},
100+
"UseCertManager without EnableWebhook": {
101+
opt: newTestOptions(func(option *Options) {
102+
option.EnableWebhook = false
103+
option.UseCertManager = true
104+
}),
105+
want: field.ErrorList{
106+
field.Invalid(newPath.Child("UseCertManager"), true, "UseCertManager requires EnableWebhook to be true (cert-manager is only used for webhook certificate management)"),
107+
field.Invalid(newPath.Child("UseCertManager"), true, "UseCertManager requires EnableWorkload to be true (cert-manager pods need to run in the hub cluster)"),
108+
},
109+
},
110+
"UseCertManager with EnableWebhook": {
111+
opt: newTestOptions(func(option *Options) {
112+
option.EnableWebhook = true
113+
option.WebhookServiceName = testWebhookServiceName
114+
option.UseCertManager = true
115+
}),
116+
want: field.ErrorList{field.Invalid(newPath.Child("UseCertManager"), true, "UseCertManager requires EnableWorkload to be true (cert-manager pods need to run in the hub cluster)")},
117+
},
118+
"UseCertManager without EnableWorkload": {
119+
opt: newTestOptions(func(option *Options) {
120+
option.EnableWebhook = true
121+
option.WebhookServiceName = testWebhookServiceName
122+
option.UseCertManager = true
123+
option.EnableWorkload = false
124+
}),
125+
want: field.ErrorList{field.Invalid(newPath.Child("UseCertManager"), true, "UseCertManager requires EnableWorkload to be true (cert-manager pods need to run in the hub cluster)")},
126+
},
127+
"UseCertManager with EnableWebhook and EnableWorkload": {
128+
opt: newTestOptions(func(option *Options) {
129+
option.EnableWebhook = true
130+
option.WebhookServiceName = testWebhookServiceName
131+
option.UseCertManager = true
132+
option.EnableWorkload = true
133+
}),
134+
want: field.ErrorList{},
135+
},
96136
}
97137

98138
for name, tc := range testCases {

‎pkg/webhook/webhook.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,11 @@ type Config struct {
165165
enableWorkload bool
166166
// useCertManager indicates whether cert-manager is used for certificate management
167167
useCertManager bool
168+
// webhookCertSecretName is the name of the Secret containing webhook certificates
169+
webhookCertSecretName string
168170
}
169171

170-
func NewWebhookConfig(mgr manager.Manager, webhookServiceName string, port int32, clientConnectionType *options.WebhookClientConnectionType, certDir string, enableGuardRail bool, denyModifyMemberClusterLabels bool, enableWorkload bool, useCertManager bool) (*Config, error) {
172+
func NewWebhookConfig(mgr manager.Manager, webhookServiceName string, port int32, clientConnectionType *options.WebhookClientConnectionType, certDir string, enableGuardRail bool, denyModifyMemberClusterLabels bool, enableWorkload bool, useCertManager bool, webhookCertSecretName string) (*Config, error) {
171173
// We assume the Pod namespace should be passed to env through downward API in the Pod spec.
172174
namespace := os.Getenv("POD_NAMESPACE")
173175
if namespace == "" {
@@ -184,6 +186,7 @@ func NewWebhookConfig(mgr manager.Manager, webhookServiceName string, port int32
184186
denyModifyMemberClusterLabels: denyModifyMemberClusterLabels,
185187
enableWorkload: enableWorkload,
186188
useCertManager: useCertManager,
189+
webhookCertSecretName: webhookCertSecretName,
187190
}
188191

189192
var caPEM []byte
@@ -238,7 +241,7 @@ func (w *Config) createMutatingWebhookConfiguration(ctx context.Context, webhook
238241
annotations := map[string]string{}
239242
if w.useCertManager {
240243
// Tell cert-manager's CA injector to automatically inject the CA bundle
241-
annotations["cert-manager.io/inject-ca-from"] = fmt.Sprintf("%s/%s", w.serviceNamespace, fleetWebhookCertSecretName)
244+
annotations["cert-manager.io/inject-ca-from"] = fmt.Sprintf("%s/%s", w.serviceNamespace, w.webhookCertSecretName)
242245
}
243246

244247
mutatingWebhookConfig := admv1.MutatingWebhookConfiguration{
@@ -298,7 +301,7 @@ func (w *Config) createValidatingWebhookConfiguration(ctx context.Context, webho
298301
annotations := map[string]string{}
299302
if w.useCertManager {
300303
// Tell cert-manager's CA injector to automatically inject the CA bundle
301-
annotations["cert-manager.io/inject-ca-from"] = fmt.Sprintf("%s/%s", w.serviceNamespace, fleetWebhookCertSecretName)
304+
annotations["cert-manager.io/inject-ca-from"] = fmt.Sprintf("%s/%s", w.serviceNamespace, w.webhookCertSecretName)
302305
}
303306

304307
validatingWebhookConfig := admv1.ValidatingWebhookConfiguration{

‎pkg/webhook/webhook_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func TestNewWebhookConfig(t *testing.T) {
194194
setupMockCertManagerFiles(t, tt.certDir)
195195
}
196196

197-
got, err := NewWebhookConfig(tt.mgr, tt.webhookServiceName, tt.port, tt.clientConnectionType, tt.certDir, tt.enableGuardRail, tt.denyModifyMemberClusterLabels, tt.enableWorkload, tt.useCertManager)
197+
got, err := NewWebhookConfig(tt.mgr, tt.webhookServiceName, tt.port, tt.clientConnectionType, tt.certDir, tt.enableGuardRail, tt.denyModifyMemberClusterLabels, tt.enableWorkload, tt.useCertManager, "fleet-webhook-server-cert")
198198
if (err != nil) != tt.wantErr {
199199
t.Errorf("NewWebhookConfig() error = %v, wantErr %v", err, tt.wantErr)
200200
return
@@ -269,7 +269,7 @@ func TestNewWebhookConfig_CertManagerNotMounted(t *testing.T) {
269269
dir := t.TempDir()
270270
// Don't create any certificate files to simulate cert-manager not ready
271271

272-
_, err := NewWebhookConfig(nil, "test-webhook", 8080, nil, dir, true, true, false, true)
272+
_, err := NewWebhookConfig(nil, "test-webhook", 8080, nil, dir, true, true, false, true, "fleet-webhook-server-cert")
273273
if err == nil {
274274
t.Error("Expected error when cert-manager certificates not mounted")
275275
}
@@ -291,10 +291,11 @@ func TestNewWebhookConfig_SelfSignedCertError(t *testing.T) {
291291
443,
292292
&clientConnectionType,
293293
invalidCertDir,
294-
false, // enableGuardRail
295-
false, // denyModifyMemberClusterLabels
296-
false, // enableWorkload
297-
false, // useCertManager = false to trigger self-signed path
294+
false, // enableGuardRail
295+
false, // denyModifyMemberClusterLabels
296+
false, // enableWorkload
297+
false, // useCertManager = false to trigger self-signed path
298+
"fleet-webhook-server-cert", // webhookCertSecretName
298299
)
299300

300301
if err == nil {

0 commit comments

Comments
 (0)