Skip to content

Commit ab38814

Browse files
andreyvelichandreyvelich
authored
Remove Namespace from the JobSet Config (#752)
* Drop namespace from JobSet Config * Fix config unit tests
1 parent ca8bb55 commit ab38814

File tree

6 files changed

+18
-89
lines changed

6 files changed

+18
-89
lines changed

‎api/config/v1alpha1/configuration_types.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,6 @@ import (
2828
type Configuration struct {
2929
metav1.TypeMeta `json:",inline"`
3030

31-
// Namespace is the namespace in which jobset controller is deployed.
32-
// It is used as part of DNSName of the webhook Service.
33-
// If not set, the value is set from the file /var/run/secrets/kubernetes.io/serviceaccount/namespace
34-
// If the file doesn't exist, default value is kueue-system.
35-
Namespace *string `json:"namespace,omitempty"`
36-
3731
// ControllerManager returns the configurations for controllers
3832
ControllerManager `json:",inline"`
3933

‎api/config/v1alpha1/defaults.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,13 @@ limitations under the License.
1717
package v1alpha1
1818

1919
import (
20-
"os"
21-
"strings"
2220
"time"
2321

2422
configv1alpha1 "k8s.io/component-base/config/v1alpha1"
2523
"k8s.io/utils/ptr"
2624
)
2725

2826
const (
29-
DefaultNamespace = "jobset-system"
3027
DefaultWebhookServiceName = "jobset-webhook-service"
3128
DefaultWebhookSecretName = "jobset-webhook-server-cert"
3229
DefaultWebhookPort = 9443
@@ -41,22 +38,10 @@ const (
4138
DefaultClientConnectionBurst int32 = 500
4239
)
4340

44-
func getOperatorNamespace() string {
45-
if data, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
46-
if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
47-
return ns
48-
}
49-
}
50-
return DefaultNamespace
51-
}
52-
5341
// SetDefaults_Configuration sets default values for ComponentConfig.
5442
//
5543
//nolint:revive // format required by generated code for defaulting
5644
func SetDefaults_Configuration(cfg *Configuration) {
57-
if cfg.Namespace == nil {
58-
cfg.Namespace = ptr.To(getOperatorNamespace())
59-
}
6045
if cfg.Webhook.Port == nil {
6146
cfg.Webhook.Port = ptr.To(DefaultWebhookPort)
6247
}

‎api/config/v1alpha1/defaults_test.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
)
2727

2828
const (
29-
overwriteNamespace = "jobset-tenant-a"
3029
overwriteWebhookPort = 9444
3130
overwriteMetricBindAddress = ":38081"
3231
overwriteHealthProbeBindAddress = ":38080"
@@ -69,7 +68,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
6968
},
7069
},
7170
want: &Configuration{
72-
Namespace: ptr.To(DefaultNamespace),
7371
ControllerManager: defaultCtrlManagerConfigurationSpec,
7472
InternalCertManagement: &InternalCertManagement{
7573
Enable: ptr.To(false),
@@ -89,7 +87,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
8987
},
9088
},
9189
want: &Configuration{
92-
Namespace: ptr.To(DefaultNamespace),
9390
ControllerManager: ControllerManager{
9491
Webhook: ControllerWebhook{
9592
Port: ptr.To(DefaultWebhookPort),
@@ -141,7 +138,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
141138
},
142139
},
143140
want: &Configuration{
144-
Namespace: ptr.To(DefaultNamespace),
145141
ControllerManager: ControllerManager{
146142
Webhook: ControllerWebhook{
147143
Port: ptr.To(overwriteWebhookPort),
@@ -179,7 +175,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
179175
},
180176
},
181177
want: &Configuration{
182-
Namespace: ptr.To(DefaultNamespace),
183178
ControllerManager: ControllerManager{
184179
Webhook: ControllerWebhook{
185180
Port: ptr.To(DefaultWebhookPort),
@@ -206,11 +201,8 @@ func TestSetDefaults_Configuration(t *testing.T) {
206201
},
207202
},
208203
"defaulting InternalCertManagement": {
209-
original: &Configuration{
210-
Namespace: ptr.To(overwriteNamespace),
211-
},
204+
original: &Configuration{},
212205
want: &Configuration{
213-
Namespace: ptr.To(overwriteNamespace),
214206
ControllerManager: defaultCtrlManagerConfigurationSpec,
215207
InternalCertManagement: &InternalCertManagement{
216208
Enable: ptr.To(true),
@@ -222,13 +214,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
222214
},
223215
"should not default InternalCertManagement": {
224216
original: &Configuration{
225-
Namespace: ptr.To(overwriteNamespace),
226217
InternalCertManagement: &InternalCertManagement{
227218
Enable: ptr.To(false),
228219
},
229220
},
230221
want: &Configuration{
231-
Namespace: ptr.To(overwriteNamespace),
232222
ControllerManager: defaultCtrlManagerConfigurationSpec,
233223
InternalCertManagement: &InternalCertManagement{
234224
Enable: ptr.To(false),
@@ -238,7 +228,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
238228
},
239229
"should not default values in custom ClientConnection": {
240230
original: &Configuration{
241-
Namespace: ptr.To(overwriteNamespace),
242231
InternalCertManagement: &InternalCertManagement{
243232
Enable: ptr.To(false),
244233
},
@@ -248,7 +237,6 @@ func TestSetDefaults_Configuration(t *testing.T) {
248237
},
249238
},
250239
want: &Configuration{
251-
Namespace: ptr.To(overwriteNamespace),
252240
ControllerManager: defaultCtrlManagerConfigurationSpec,
253241
InternalCertManagement: &InternalCertManagement{
254242
Enable: ptr.To(false),
@@ -261,14 +249,12 @@ func TestSetDefaults_Configuration(t *testing.T) {
261249
},
262250
"should default empty custom ClientConnection": {
263251
original: &Configuration{
264-
Namespace: ptr.To(overwriteNamespace),
265252
InternalCertManagement: &InternalCertManagement{
266253
Enable: ptr.To(false),
267254
},
268255
ClientConnection: &ClientConnection{},
269256
},
270257
want: &Configuration{
271-
Namespace: ptr.To(overwriteNamespace),
272258
ControllerManager: defaultCtrlManagerConfigurationSpec,
273259
InternalCertManagement: &InternalCertManagement{
274260
Enable: ptr.To(false),

‎api/config/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/config/config_test.go

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,10 @@ func TestLoad(t *testing.T) {
4848

4949
tmpDir := t.TempDir()
5050

51-
namespaceOverWriteConfig := filepath.Join(tmpDir, "namespace-overwrite.yaml")
52-
if err := os.WriteFile(namespaceOverWriteConfig, []byte(`
53-
apiVersion: config.jobset.x-k8s.io/v1alpha1
54-
kind: Configuration
55-
namespace: jobset-tenant-a
56-
health:
57-
healthProbeBindAddress: :8081
58-
metrics:
59-
bindAddress: :8080
60-
leaderElection:
61-
leaderElect: true
62-
resourceName: 6d4f6a47.jobset.x-k8s.io
63-
webhook:
64-
port: 9443
65-
`), os.FileMode(0600)); err != nil {
66-
t.Fatal(err)
67-
}
68-
6951
ctrlManagerConfigSpecOverWriteConfig := filepath.Join(tmpDir, "ctrl-manager-config-spec-overwrite.yaml")
7052
if err := os.WriteFile(ctrlManagerConfigSpecOverWriteConfig, []byte(`
7153
apiVersion: config.jobset.x-k8s.io/v1alpha1
7254
kind: Configuration
73-
namespace: jobset-system
7455
health:
7556
healthProbeBindAddress: :38081
7657
metrics:
@@ -88,7 +69,6 @@ webhook:
8869
if err := os.WriteFile(certOverWriteConfig, []byte(`
8970
apiVersion: config.jobset.x-k8s.io/v1alpha1
9071
kind: Configuration
91-
namespace: jobset-system
9272
health:
9373
healthProbeBindAddress: :8081
9474
metrics:
@@ -110,7 +90,6 @@ internalCertManagement:
11090
if err := os.WriteFile(disableCertOverWriteConfig, []byte(`
11191
apiVersion: config.jobset.x-k8s.io/v1alpha1
11292
kind: Configuration
113-
namespace: jobset-system
11493
health:
11594
healthProbeBindAddress: :8081
11695
metrics:
@@ -130,7 +109,6 @@ internalCertManagement:
130109
if err := os.WriteFile(leaderElectionDisabledConfig, []byte(`
131110
apiVersion: config.jobset.x-k8s.io/v1alpha1
132111
kind: Configuration
133-
namespace: jobset-system
134112
health:
135113
healthProbeBindAddress: :8081
136114
metrics:
@@ -147,7 +125,6 @@ webhook:
147125
if err := os.WriteFile(clientConnectionConfig, []byte(`
148126
apiVersion: config.jobset.x-k8s.io/v1alpha1
149127
kind: Configuration
150-
namespace: jobset-system
151128
health:
152129
healthProbeBindAddress: :8081
153130
metrics:
@@ -168,7 +145,6 @@ clientConnection:
168145
if err := os.WriteFile(invalidConfig, []byte(`
169146
apiVersion: config.jobset.x-k8s.io/v1alpha1
170147
kind: Configuration
171-
namespaces: jobset-system
172148
invalidField: invalidValue
173149
health:
174150
healthProbeBindAddress: :8081
@@ -237,7 +213,6 @@ webhook:
237213
name: "default config",
238214
configFile: "",
239215
wantConfiguration: configapi.Configuration{
240-
Namespace: ptr.To(configapi.DefaultNamespace),
241216
InternalCertManagement: enableDefaultInternalCertManagement,
242217
ClientConnection: defaultClientConnection,
243218
},
@@ -268,20 +243,6 @@ webhook:
268243
Err: errors.New("is a directory"),
269244
},
270245
},
271-
{
272-
name: "namespace overwrite config",
273-
configFile: namespaceOverWriteConfig,
274-
wantConfiguration: configapi.Configuration{
275-
TypeMeta: metav1.TypeMeta{
276-
APIVersion: configapi.GroupVersion.String(),
277-
Kind: "Configuration",
278-
},
279-
Namespace: ptr.To("jobset-tenant-a"),
280-
InternalCertManagement: enableDefaultInternalCertManagement,
281-
ClientConnection: defaultClientConnection,
282-
},
283-
wantOptions: defaultControlOptions,
284-
},
285246
{
286247
name: "ControllerManagerConfigurationSpec overwrite config",
287248
configFile: ctrlManagerConfigSpecOverWriteConfig,
@@ -290,7 +251,6 @@ webhook:
290251
APIVersion: configapi.GroupVersion.String(),
291252
Kind: "Configuration",
292253
},
293-
Namespace: ptr.To(configapi.DefaultNamespace),
294254
InternalCertManagement: enableDefaultInternalCertManagement,
295255
ClientConnection: defaultClientConnection,
296256
},
@@ -320,7 +280,6 @@ webhook:
320280
APIVersion: configapi.GroupVersion.String(),
321281
Kind: "Configuration",
322282
},
323-
Namespace: ptr.To(configapi.DefaultNamespace),
324283
InternalCertManagement: &configapi.InternalCertManagement{
325284
Enable: ptr.To(true),
326285
WebhookServiceName: ptr.To("jobset-tenant-a-webhook-service"),
@@ -338,7 +297,6 @@ webhook:
338297
APIVersion: configapi.GroupVersion.String(),
339298
Kind: "Configuration",
340299
},
341-
Namespace: ptr.To(configapi.DefaultNamespace),
342300
InternalCertManagement: &configapi.InternalCertManagement{
343301
Enable: ptr.To(false),
344302
},
@@ -354,7 +312,6 @@ webhook:
354312
APIVersion: configapi.GroupVersion.String(),
355313
Kind: "Configuration",
356314
},
357-
Namespace: ptr.To("jobset-system"),
358315
InternalCertManagement: enableDefaultInternalCertManagement,
359316
ClientConnection: defaultClientConnection,
360317
},
@@ -384,7 +341,6 @@ webhook:
384341
APIVersion: configapi.GroupVersion.String(),
385342
Kind: "Configuration",
386343
},
387-
Namespace: ptr.To(configapi.DefaultNamespace),
388344
InternalCertManagement: enableDefaultInternalCertManagement,
389345
ClientConnection: &configapi.ClientConnection{
390346
QPS: ptr.To[float32](50),
@@ -398,7 +354,6 @@ webhook:
398354
configFile: invalidConfig,
399355
wantError: runtime.NewStrictDecodingError([]error{
400356
errors.New("unknown field \"invalidField\""),
401-
errors.New("unknown field \"namespaces\""),
402357
}),
403358
},
404359
}
@@ -461,7 +416,6 @@ func TestEncode(t *testing.T) {
461416
wantResult: map[string]any{
462417
"apiVersion": "config.jobset.x-k8s.io/v1alpha1",
463418
"kind": "Configuration",
464-
"namespace": configapi.DefaultNamespace,
465419
"webhook": map[string]any{
466420
"port": int64(configapi.DefaultWebhookPort),
467421
},

‎pkg/util/cert/cert.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ package cert
1515

1616
import (
1717
"fmt"
18+
"os"
19+
"strings"
1820

1921
cert "github.com/open-policy-agent/cert-controller/pkg/rotator"
2022
"k8s.io/apimachinery/pkg/types"
@@ -24,24 +26,37 @@ import (
2426
)
2527

2628
const (
29+
defaultNamespace = "jobset-system"
2730
certDir = "/tmp/k8s-webhook-server/serving-certs"
2831
validateWebhookConfName = "jobset-validating-webhook-configuration"
2932
mutatingWebhookConfName = "jobset-mutating-webhook-configuration"
3033
caName = "jobset-ca"
3134
caOrg = "jobset"
3235
)
3336

37+
func getOperatorNamespace() string {
38+
if data, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
39+
if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
40+
return ns
41+
}
42+
}
43+
return defaultNamespace
44+
}
45+
3446
//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update
3547
//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=mutatingwebhookconfigurations,verbs=get;list;watch;update
3648
//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingwebhookconfigurations,verbs=get;list;watch;update
3749

3850
// CertsManager creates certs for webhooks.
3951
func CertsManager(mgr ctrl.Manager, cfg config.Configuration, setupFinish chan struct{}) error {
52+
// Webhook and controller must be deployed in the same namespace.
53+
namespace := getOperatorNamespace()
54+
4055
// DNSName is <service name>.<namespace>.svc
41-
var dnsName = fmt.Sprintf("%s.%s.svc", *cfg.InternalCertManagement.WebhookServiceName, *cfg.Namespace)
56+
var dnsName = fmt.Sprintf("%s.%s.svc", *cfg.InternalCertManagement.WebhookServiceName, namespace)
4257
return cert.AddRotator(mgr, &cert.CertRotator{
4358
SecretKey: types.NamespacedName{
44-
Namespace: *cfg.Namespace,
59+
Namespace: namespace,
4560
Name: *cfg.InternalCertManagement.WebhookSecretName,
4661
},
4762
CertDir: certDir,

0 commit comments

Comments
 (0)