Skip to content

Commit ced5993

Browse files
skattojuskattoju
committed
Use SA from spec
1 parent 40e1bac commit ced5993

16 files changed

+439
-73
lines changed

‎Makefile

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,16 +188,18 @@ kind-clean: $(KIND) #EXHELP Delete the kind cluster.
188188

189189
.PHONY: kind-load-test-artifacts
190190
kind-load-test-artifacts: $(KIND) #EXHELP Load the e2e testdata container images into a kind cluster.
191-
$(CONTAINER_RUNTIME) build testdata/bundles/registry-v1/prometheus-operator.v1.0.0 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0
191+
$(CONTAINER_RUNTIME) build testdata/bundles/registry-v1/prometheus-operator.v1.0.0 -t localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 --load
192192
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1
193193
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0
194194
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0
195195
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 --name $(KIND_CLUSTER_NAME)
196196
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1 --name $(KIND_CLUSTER_NAME)
197197
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0 --name $(KIND_CLUSTER_NAME)
198198
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0 --name $(KIND_CLUSTER_NAME)
199-
$(CONTAINER_RUNTIME) build testdata/bundles/registry-v1/package-with-webhooks.v1.0.0 -t localhost/testdata/bundles/registry-v1/package-with-webhooks:v1.0.0
199+
$(CONTAINER_RUNTIME) build testdata/bundles/registry-v1/package-with-webhooks.v1.0.0 -t localhost/testdata/bundles/registry-v1/package-with-webhooks:v1.0.0 --load
200200
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/package-with-webhooks:v1.0.0 --name $(KIND_CLUSTER_NAME)
201+
kubectl apply -f testdata/rbac/prometheus-operator-bundle-rbac.yaml -n default
202+
kubectl apply -f testdata/rbac/package-with-webhooks-rbac.yaml -n default
201203

202204

203205
#SECTION Build
@@ -245,7 +247,7 @@ run: docker-build kind-cluster kind-load kind-deploy #HELP Build the operator-co
245247

246248
.PHONY: docker-build
247249
docker-build: build-linux #EXHELP Build docker image for operator-controller with GOOS=linux and local GOARCH.
248-
$(CONTAINER_RUNTIME) build -t $(IMG) -f Dockerfile ./bin/linux
250+
$(CONTAINER_RUNTIME) build -t $(IMG) -f Dockerfile ./bin/linux --load
249251

250252
#SECTION Release
251253
ifeq ($(origin ENABLE_RELEASE_PIPELINE), undefined)

‎api/v1alpha1/clusterextension_types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,12 @@ type ClusterExtensionSpec struct {
7878
// the bundle may contain resources that are cluster-scoped or that are
7979
// installed in a different namespace. This namespace is expected to exist.
8080
InstallNamespace string `json:"installNamespace"`
81+
82+
//+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$
83+
//+kubebuilder:validation:MaxLength:=253
84+
// ServiceAccountName is the name of the ServiceAccount to use to manage the resources in the bundle.
85+
// The service account is expected to exist in the InstallNamespace.
86+
ServiceAccountName string `json:"serviceAccountName"`
8187
}
8288

8389
const (

‎cmd/manager/main.go

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package main
1818

1919
import (
20+
"context"
2021
"crypto/x509"
2122
"flag"
2223
"fmt"
@@ -29,7 +30,10 @@ import (
2930
"go.uber.org/zap/zapcore"
3031
k8slabels "k8s.io/apimachinery/pkg/labels"
3132
"k8s.io/apimachinery/pkg/selection"
33+
"k8s.io/apimachinery/pkg/types"
34+
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
3235
_ "k8s.io/client-go/plugin/pkg/client/auth"
36+
"k8s.io/client-go/rest"
3337
ctrl "sigs.k8s.io/controller-runtime"
3438
crcache "sigs.k8s.io/controller-runtime/pkg/cache"
3539
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -42,6 +46,7 @@ import (
4246
"github.com/operator-framework/rukpak/pkg/storage"
4347

4448
"github.com/operator-framework/operator-controller/api/v1alpha1"
49+
"github.com/operator-framework/operator-controller/internal/authentication"
4550
"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
4651
catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
4752
"github.com/operator-framework/operator-controller/internal/controllers"
@@ -155,14 +160,44 @@ func main() {
155160
cl := mgr.GetClient()
156161
catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, &http.Client{Timeout: 10 * time.Second}))
157162

158-
cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), helmclient.StorageNamespaceMapper(func(o client.Object) (string, error) {
159-
return systemNamespace, nil
160-
}))
163+
saGetter, err := corev1client.NewForConfig(ctrl.GetConfigOrDie())
161164
if err != nil {
162-
setupLog.Error(err, "unable to config for creating helm client")
165+
setupLog.Error(err, "unable to create service account client")
163166
os.Exit(1)
164167
}
165168

169+
tg := authentication.NewTokenGetter(saGetter, 3600)
170+
nsMapper := func(obj client.Object) (string, error) {
171+
bd, ok := obj.(*v1alpha1.ClusterExtension)
172+
if !ok {
173+
return "", fmt.Errorf("cannot derive namespace from object of type %T", obj)
174+
}
175+
return bd.Spec.InstallNamespace, nil
176+
}
177+
178+
rcm := func(ctx context.Context, obj client.Object, baseRestConfig *rest.Config) (*rest.Config, error) {
179+
cfg := rest.AnonymousClientConfig(rest.CopyConfig(baseRestConfig))
180+
ext, ok := obj.(*v1alpha1.ClusterExtension)
181+
if !ok {
182+
return cfg, nil
183+
}
184+
token, err := tg.Get(ctx, types.NamespacedName{Namespace: ext.Spec.InstallNamespace, Name: ext.Spec.ServiceAccountName})
185+
if err != nil {
186+
return nil, err
187+
}
188+
cfg.BearerToken = token
189+
return cfg, nil
190+
}
191+
192+
cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
193+
helmclient.ClientNamespaceMapper(nsMapper),
194+
helmclient.StorageNamespaceMapper(nsMapper),
195+
helmclient.RestConfigMapper(rcm),
196+
)
197+
if err != nil {
198+
setupLog.Error(err, "unable to config for creating helm client")
199+
os.Exit(1)
200+
}
166201
acg, err := helmclient.NewActionClientGetter(cfgGetter)
167202
if err != nil {
168203
setupLog.Error(err, "unable to create helm client")

‎config/crd/bases/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ spec:
5656
maxLength: 48
5757
pattern: ^[a-z0-9]+(-[a-z0-9]+)*$
5858
type: string
59+
serviceAccountName:
60+
description: |-
61+
ServiceAccountName is the name of the ServiceAccount to use to manage the resources in the bundle.
62+
The service account is expected to exist in the InstallNamespace.
63+
maxLength: 253
64+
pattern: ^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$
65+
type: string
5966
upgradeConstraintPolicy:
6067
default: Enforce
6168
description: Defines the policy for how to handle upgrade constraints
@@ -77,6 +84,7 @@ spec:
7784
required:
7885
- installNamespace
7986
- packageName
87+
- serviceAccountName
8088
type: object
8189
status:
8290
description: ClusterExtensionStatus defines the observed state of ClusterExtension

‎config/samples/olm_v1alpha1_clusterextension.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ metadata:
44
name: clusterextension-sample
55
spec:
66
installNamespace: default
7+
serviceAccountName: default
78
packageName: argocd-operator
89
version: 0.6.0

‎internal/authentication/tokengetter.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package authentication
2+
3+
import (
4+
"context"
5+
"sync"
6+
"time"
7+
8+
authenticationv1 "k8s.io/api/authentication/v1"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
"k8s.io/apimachinery/pkg/types"
11+
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
12+
"k8s.io/utils/ptr"
13+
)
14+
15+
type TokenGetter struct {
16+
client corev1client.ServiceAccountsGetter
17+
expirationSeconds int64
18+
tokens map[types.NamespacedName]*authenticationv1.TokenRequestStatus
19+
tokenLocks keyLock[types.NamespacedName]
20+
mu sync.RWMutex
21+
}
22+
23+
func NewTokenGetter(client corev1client.ServiceAccountsGetter, expirationSeconds int64) *TokenGetter {
24+
return &TokenGetter{
25+
client: client,
26+
expirationSeconds: expirationSeconds,
27+
tokenLocks: newKeyLock[types.NamespacedName](),
28+
tokens: map[types.NamespacedName]*authenticationv1.TokenRequestStatus{},
29+
}
30+
}
31+
32+
type keyLock[K comparable] struct {
33+
locks map[K]*sync.Mutex
34+
mu sync.Mutex
35+
}
36+
37+
func newKeyLock[K comparable]() keyLock[K] {
38+
return keyLock[K]{locks: map[K]*sync.Mutex{}}
39+
}
40+
41+
func (k *keyLock[K]) Lock(key K) {
42+
k.getLock(key).Lock()
43+
}
44+
45+
func (k *keyLock[K]) Unlock(key K) {
46+
k.getLock(key).Unlock()
47+
}
48+
49+
func (k *keyLock[K]) getLock(key K) *sync.Mutex {
50+
k.mu.Lock()
51+
defer k.mu.Unlock()
52+
53+
lock, ok := k.locks[key]
54+
if !ok {
55+
lock = &sync.Mutex{}
56+
k.locks[key] = lock
57+
}
58+
return lock
59+
}
60+
61+
func (t *TokenGetter) Get(ctx context.Context, key types.NamespacedName) (string, error) {
62+
t.tokenLocks.Lock(key)
63+
defer t.tokenLocks.Unlock(key)
64+
65+
t.mu.RLock()
66+
token, ok := t.tokens[key]
67+
t.mu.RUnlock()
68+
69+
expireTime := time.Time{}
70+
if ok {
71+
expireTime = token.ExpirationTimestamp.Time
72+
}
73+
74+
fiveMinutesAfterNow := metav1.Now().Add(5 * time.Minute)
75+
if expireTime.Before(fiveMinutesAfterNow) {
76+
var err error
77+
token, err = t.getToken(ctx, key)
78+
if err != nil {
79+
return "", err
80+
}
81+
t.mu.Lock()
82+
t.tokens[key] = token
83+
t.mu.Unlock()
84+
}
85+
86+
return token.Token, nil
87+
}
88+
89+
func (t *TokenGetter) getToken(ctx context.Context, key types.NamespacedName) (*authenticationv1.TokenRequestStatus, error) {
90+
req, err := t.client.ServiceAccounts(key.Namespace).CreateToken(ctx, key.Name, &authenticationv1.TokenRequest{Spec: authenticationv1.TokenRequestSpec{
91+
ExpirationSeconds: ptr.To[int64](3600),
92+
}}, metav1.CreateOptions{})
93+
if err != nil {
94+
return nil, err
95+
}
96+
return &req.Status, nil
97+
}

‎internal/controllers/clusterextension_admission_test.go

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) {
4343
t.Parallel()
4444
cl := newClient(t)
4545
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
46-
PackageName: tc.pkgName,
47-
InstallNamespace: "default",
46+
PackageName: tc.pkgName,
47+
InstallNamespace: "default",
48+
ServiceAccountName: "default",
4849
}))
4950
if tc.errMsg == "" {
5051
require.NoError(t, err, "unexpected error for package name %q: %w", tc.pkgName, err)
@@ -131,9 +132,10 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) {
131132
t.Parallel()
132133
cl := newClient(t)
133134
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
134-
PackageName: "package",
135-
Version: tc.version,
136-
InstallNamespace: "default",
135+
PackageName: "package",
136+
Version: tc.version,
137+
InstallNamespace: "default",
138+
ServiceAccountName: "default",
137139
}))
138140
if tc.errMsg == "" {
139141
require.NoError(t, err, "unexpected error for version %q: %w", tc.version, err)
@@ -176,9 +178,10 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) {
176178
t.Parallel()
177179
cl := newClient(t)
178180
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
179-
PackageName: "package",
180-
Channel: tc.channelName,
181-
InstallNamespace: "default",
181+
PackageName: "package",
182+
Channel: tc.channelName,
183+
InstallNamespace: "default",
184+
ServiceAccountName: "default",
182185
}))
183186
if tc.errMsg == "" {
184187
require.NoError(t, err, "unexpected error for channel %q: %w", tc.channelName, err)
@@ -222,8 +225,9 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
222225
t.Parallel()
223226
cl := newClient(t)
224227
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
225-
PackageName: "package",
226-
InstallNamespace: tc.installNamespace,
228+
PackageName: "package",
229+
InstallNamespace: tc.installNamespace,
230+
ServiceAccountName: "default",
227231
}))
228232
if tc.errMsg == "" {
229233
require.NoError(t, err, "unexpected error for installNamespace %q: %w", tc.installNamespace, err)
@@ -235,6 +239,51 @@ func TestClusterExtensionAdmissionInstallNamespace(t *testing.T) {
235239
}
236240
}
237241

242+
func TestClusterExtensionAdmissionServiceAccountName(t *testing.T) {
243+
tooLongError := "spec.serviceAccountName: Too long: may not be longer than 253"
244+
regexMismatchError := "spec.serviceAccountName in body should match"
245+
246+
testCases := []struct {
247+
name string
248+
serviceAccountName string
249+
errMsg string
250+
}{
251+
{"just alphanumeric", "justalphanumberic1", ""},
252+
{"hypen-separated", "hyphenated-name", ""},
253+
{"no service account name", "", regexMismatchError},
254+
{"longest valid service account name", strings.Repeat("x", 253), ""},
255+
{"too long service account name", strings.Repeat("x", 254), tooLongError},
256+
{"spaces", "spaces spaces", regexMismatchError},
257+
{"capitalized", "Capitalized", regexMismatchError},
258+
{"camel case", "camelCase", regexMismatchError},
259+
{"invalid characters", "many/invalid$characters+in_name", regexMismatchError},
260+
{"starts with hyphen", "-start-with-hyphen", regexMismatchError},
261+
{"ends with hyphen", "end-with-hyphen-", regexMismatchError},
262+
{"starts with period", ".start-with-period", regexMismatchError},
263+
{"ends with period", "end-with-period.", regexMismatchError},
264+
}
265+
266+
t.Parallel()
267+
for _, tc := range testCases {
268+
tc := tc
269+
t.Run(tc.name, func(t *testing.T) {
270+
t.Parallel()
271+
cl := newClient(t)
272+
err := cl.Create(context.Background(), buildClusterExtension(ocv1alpha1.ClusterExtensionSpec{
273+
PackageName: "package",
274+
InstallNamespace: "default",
275+
ServiceAccountName: tc.serviceAccountName,
276+
}))
277+
if tc.errMsg == "" {
278+
require.NoError(t, err, "unexpected error for serviceAccountName %q: %w", tc.serviceAccountName, err)
279+
} else {
280+
require.Error(t, err)
281+
require.Contains(t, err.Error(), tc.errMsg)
282+
}
283+
})
284+
}
285+
}
286+
238287
func buildClusterExtension(spec ocv1alpha1.ClusterExtensionSpec) *ocv1alpha1.ClusterExtension {
239288
return &ocv1alpha1.ClusterExtension{
240289
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)