Skip to content

Commit 632b43d

Browse files
authored
implement RBAC check in AdmissionController (#25)
1. Implement RBAC check in AdmissionController 2. Write unit tests for AdmissionController 3. Enabled unit tests in CI
1 parent 13bae6b commit 632b43d

16 files changed

+521
-207
lines changed

.github/workflows/CI.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ jobs:
4444
run: |
4545
make docker-build -e GIT_BRANCH=${{ env.GIT_BRANCH }} TAG=${{ env.GIT_BRANCH }}-${{ env.TAG }}
4646
47+
- name: Run Unit Tests
48+
run: make test
49+
4750
- name: Create and configure cluster
4851
run: ./hack/create-test-cluster.sh
4952

Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ vet: ## Run go vet against code.
8888

8989
.PHONY: test
9090
test: manifests generate fmt vet envtest ## Run unit tests.
91-
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
91+
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -v -ginkgo.v -coverprofile cover.out
9292

9393
# Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors.
9494
.PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up.

cmd/main.go

+5-6
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ func main() {
7575
flag.BoolVar(&enableHTTP2, "enable-http2", false,
7676
"If set, HTTP/2 will be enabled for the metrics and webhook servers")
7777
flag.BoolVar(&config.ManageJobsWithoutQueueName, "manage-no-queue", true, "Manage AppWrappers without queue names")
78+
flag.StringVar(&config.ServiceAccountName, "service-account", "", "Service account name for controller")
7879
opts := zap.Options{
7980
Development: true,
8081
}
@@ -83,6 +84,7 @@ func main() {
8384

8485
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
8586
setupLog.Info("Build info", "version", BuildVersion, "date", BuildDate)
87+
setupLog.Info("Configuration", "config", config)
8688

8789
// if the enable-http2 flag is false (the default), http/2 should be disabled
8890
// due to its vulnerabilities. More specifically, disabling http/2 will
@@ -150,12 +152,9 @@ func main() {
150152
os.Exit(1)
151153
}
152154
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
153-
wh := &controller.AppWrapperWebhook{Config: &config}
154-
if err := ctrl.NewWebhookManagedBy(mgr).
155-
For(&workloadv1beta2.AppWrapper{}).
156-
WithDefaulter(wh).
157-
WithValidator(wh).
158-
Complete(); err != nil {
155+
if err = (&controller.AppWrapperWebhook{
156+
Config: &config,
157+
}).SetupWebhookWithManager(mgr); err != nil {
159158
setupLog.Error(err, "unable to create webhook", "webhook", "AppWrapper")
160159
os.Exit(1)
161160
}

config/default/manager_auth_proxy_patch.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ spec:
3434
memory: 64Mi
3535
- name: manager
3636
args:
37+
- "--service-account=system:serviceaccount:$(MY_NAMESPACE):$(MY_SERVICE_ACCOUNT_NAME)"
3738
- "--health-probe-bind-address=:8081"
3839
- "--metrics-bind-address=127.0.0.1:8080"
3940
- "--leader-elect"

config/manager/manager.yaml

+10-2
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ spec:
7070
- /manager
7171
args:
7272
- --leader-elect
73+
- "--service-account=system:serviceaccount:$(MY_NAMESPACE):$(MY_SERVICE_ACCOUNT_NAME)"
7374
image: controller:latest
7475
name: manager
7576
securityContext:
@@ -89,14 +90,21 @@ spec:
8990
port: 8081
9091
initialDelaySeconds: 5
9192
periodSeconds: 10
92-
# TODO(user): Configure the resources accordingly based on the project requirements.
93-
# More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
9493
resources:
9594
limits:
9695
cpu: 500m
9796
memory: 128Mi
9897
requests:
9998
cpu: 10m
10099
memory: 64Mi
100+
env:
101+
- name: MY_SERVICE_ACCOUNT_NAME
102+
valueFrom:
103+
fieldRef:
104+
fieldPath: spec.serviceAccountName
105+
- name: MY_NAMESPACE
106+
valueFrom:
107+
fieldRef:
108+
fieldPath: metadata.namespace
101109
serviceAccountName: controller-manager
102110
terminationGracePeriodSeconds: 10

config/rbac/role.yaml

+12
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ rules:
2626
- patch
2727
- update
2828
- watch
29+
- apiGroups:
30+
- apiextensions.k8s.io
31+
resources:
32+
- customresourcedefinitions
33+
verbs:
34+
- list
2935
- apiGroups:
3036
- apps
3137
resources:
@@ -39,6 +45,12 @@ rules:
3945
- patch
4046
- update
4147
- watch
48+
- apiGroups:
49+
- authorization.k8s.io
50+
resources:
51+
- subjectaccessreviews
52+
verbs:
53+
- create
4254
- apiGroups:
4355
- batch
4456
resources:

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ require (
1010
k8s.io/client-go v0.29.1
1111
sigs.k8s.io/controller-runtime v0.17.0
1212
sigs.k8s.io/kueue v0.6.0
13+
sigs.k8s.io/yaml v1.4.0
1314
)
1415

1516
require (
@@ -73,5 +74,4 @@ require (
7374
sigs.k8s.io/jobset v0.3.1 // indirect
7475
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
7576
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
76-
sigs.k8s.io/yaml v1.4.0 // indirect
7777
)

internal/controller/confg.go renamed to internal/controller/appwrapper_config.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ limitations under the License.
1717
package controller
1818

1919
type AppWrapperConfig struct {
20-
ManageJobsWithoutQueueName bool `json:"manageJobsWithoutQueueName,omitempty"`
20+
ManageJobsWithoutQueueName bool `json:"manageJobsWithoutQueueName,omitempty"`
21+
ServiceAccountName string `json:"serviceAccountName,omitempty"`
2122
}

internal/controller/appwrapper_controller_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ limitations under the License.
1616

1717
package controller
1818

19+
/*
20+
1921
import (
2022
"context"
2123
@@ -82,3 +84,5 @@ var _ = Describe("AppWrapper Controller", func() {
8284
})
8385
})
8486
})
87+
88+
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
/*
2+
Copyright 2024 IBM Corporation.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller
18+
19+
import (
20+
"fmt"
21+
"math/rand"
22+
"time"
23+
24+
. "github.com/onsi/gomega"
25+
26+
workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
27+
"k8s.io/apimachinery/pkg/api/resource"
28+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
"k8s.io/apimachinery/pkg/runtime"
30+
"sigs.k8s.io/yaml"
31+
)
32+
33+
const charset = "abcdefghijklmnopqrstuvwxyz0123456789"
34+
35+
func randName(baseName string) string {
36+
seededRand := rand.New(rand.NewSource(time.Now().UnixNano()))
37+
b := make([]byte, 6)
38+
for i := range b {
39+
b[i] = charset[seededRand.Intn(len(charset))]
40+
}
41+
return fmt.Sprintf("%s-%s", baseName, string(b))
42+
}
43+
44+
func toAppWrapper(components ...workloadv1beta2.AppWrapperComponent) *workloadv1beta2.AppWrapper {
45+
return &workloadv1beta2.AppWrapper{
46+
ObjectMeta: metav1.ObjectMeta{Name: randName("aw"), Namespace: "default"},
47+
Spec: workloadv1beta2.AppWrapperSpec{Components: components},
48+
}
49+
}
50+
51+
const podYAML = `
52+
apiVersion: v1
53+
kind: Pod
54+
metadata:
55+
name: %v
56+
spec:
57+
restartPolicy: Never
58+
containers:
59+
- name: busybox
60+
image: quay.io/project-codeflare/busybox:1.36
61+
command: ["sh", "-c", "sleep 1000"]
62+
resources:
63+
requests:
64+
cpu: %v`
65+
66+
func pod(milliCPU int64) workloadv1beta2.AppWrapperComponent {
67+
yamlString := fmt.Sprintf(podYAML,
68+
randName("pod"),
69+
resource.NewMilliQuantity(milliCPU, resource.DecimalSI))
70+
71+
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
72+
Expect(err).NotTo(HaveOccurred())
73+
replicas := int32(1)
74+
return workloadv1beta2.AppWrapperComponent{
75+
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: &replicas, Path: "template"}},
76+
Template: runtime.RawExtension{Raw: jsonBytes},
77+
}
78+
}
79+
80+
const namespacedPodYAML = `
81+
apiVersion: v1
82+
kind: Pod
83+
metadata:
84+
name: %v
85+
namespace: %v
86+
spec:
87+
restartPolicy: Never
88+
containers:
89+
- name: busybox
90+
image: quay.io/project-codeflare/busybox:1.36
91+
command: ["sh", "-c", "sleep 1000"]
92+
resources:
93+
requests:
94+
cpu: %v`
95+
96+
func namespacedPod(namespace string, milliCPU int64) workloadv1beta2.AppWrapperComponent {
97+
yamlString := fmt.Sprintf(namespacedPodYAML,
98+
randName("pod"),
99+
namespace,
100+
resource.NewMilliQuantity(milliCPU, resource.DecimalSI))
101+
102+
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
103+
Expect(err).NotTo(HaveOccurred())
104+
replicas := int32(1)
105+
return workloadv1beta2.AppWrapperComponent{
106+
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: &replicas, Path: "template"}},
107+
Template: runtime.RawExtension{Raw: jsonBytes},
108+
}
109+
}
110+
111+
const serviceYAML = `
112+
apiVersion: v1
113+
kind: Service
114+
metadata:
115+
name: %v
116+
spec:
117+
selector:
118+
app: test
119+
ports:
120+
- protocol: TCP
121+
port: 80
122+
targetPort: 8080`
123+
124+
func service() workloadv1beta2.AppWrapperComponent {
125+
yamlString := fmt.Sprintf(serviceYAML, randName("service"))
126+
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
127+
Expect(err).NotTo(HaveOccurred())
128+
return workloadv1beta2.AppWrapperComponent{
129+
PodSets: []workloadv1beta2.AppWrapperPodSet{},
130+
Template: runtime.RawExtension{Raw: jsonBytes},
131+
}
132+
}
133+
134+
const deploymentYAML = `
135+
apiVersion: apps/v1
136+
kind: Deployment
137+
metadata:
138+
name: %v
139+
labels:
140+
app: test
141+
spec:
142+
replicas: %v
143+
selector:
144+
matchLabels:
145+
app: test
146+
template:
147+
metadata:
148+
labels:
149+
app: test
150+
spec:
151+
terminationGracePeriodSeconds: 0
152+
containers:
153+
- name: busybox
154+
image: quay.io/project-codeflare/busybox:1.36
155+
command: ["sh", "-c", "sleep 10000"]
156+
resources:
157+
requests:
158+
cpu: %v`
159+
160+
func deployment(replicaCount int, milliCPU int64) workloadv1beta2.AppWrapperComponent {
161+
yamlString := fmt.Sprintf(deploymentYAML,
162+
randName("deployment"),
163+
replicaCount,
164+
resource.NewMilliQuantity(milliCPU, resource.DecimalSI))
165+
166+
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
167+
Expect(err).NotTo(HaveOccurred())
168+
replicas := int32(replicaCount)
169+
return workloadv1beta2.AppWrapperComponent{
170+
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: &replicas, Path: "template.spec.template"}},
171+
Template: runtime.RawExtension{Raw: jsonBytes},
172+
}
173+
}

0 commit comments

Comments
 (0)