Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix!: remove list and watch on secrets. Fixes #8534 #8555

Merged
merged 23 commits into from May 13, 2022
Merged

fix!: remove list and watch on secrets. Fixes #8534 #8555

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e6afd37
fix: remove secrets
Apr 30, 2022
c06ea3e
feat: remove list and watch on secrets
Apr 30, 2022
65ac3e9
fix: lint
May 1, 2022
e096ce8
feat: initial draft of timed cache
May 2, 2022
fa5a098
feat: initial draft of timed cache
May 2, 2022
7a56851
feat: merge
May 2, 2022
ada3ef4
feat: add tests
May 2, 2022
c8772ec
feat: merge
May 3, 2022
41830bd
fix: remove generics to fix build
May 3, 2022
83ed08b
feat: rename variables
May 3, 2022
1155102
feat: rename variables
May 3, 2022
901ea65
feat: rename variables
May 3, 2022
8b0b362
fix: remove sso-namespace option
May 4, 2022
56e064c
fix: set cache TTL to 1m
May 4, 2022
0e57a32
fix: docs
May 4, 2022
22e6bdb
feat: fix namespaces
May 4, 2022
17a649f
fix: rename variable
May 4, 2022
629f3dd
fix: minor
May 5, 2022
edc69b3
feat: start informerFactory in a separate method
May 5, 2022
56fbaaf
feat: fix time tests
May 5, 2022
e232df4
fix: resolve ssonamespace in
May 5, 2022
91271eb
fix: pass ctx as function argument
May 6, 2022
d0541c1
fix: pass ctx as function argument
May 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions manifests/cluster-install/argo-server-rbac/argo-server-clusterole.yaml
View file
Open in desktop
Expand Up @@ -18,8 +18,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/install.yaml
View file
Open in desktop
Expand Up @@ -944,8 +944,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/namespace-install.yaml
View file
Open in desktop
Expand Up @@ -853,8 +853,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/namespace-install/argo-server-rbac/argo-server-role.yaml
View file
Open in desktop
Expand Up @@ -18,8 +18,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/quick-start-minimal.yaml
View file
Open in desktop
Expand Up @@ -882,8 +882,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/quick-start-mysql.yaml
View file
Open in desktop
Expand Up @@ -882,8 +882,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 0 additions & 2 deletions manifests/quick-start-postgres.yaml
View file
Open in desktop
Expand Up @@ -882,8 +882,6 @@ rules:
verbs:
- get
- create
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 1 addition & 1 deletion server/auth/gatekeeper.go
View file
Open in desktop
Expand Up @@ -318,7 +318,7 @@ func (s *gatekeeper) authorizationForServiceAccount(serviceAccount *corev1.Servi
if len(serviceAccount.Secrets) == 0 {
return "", fmt.Errorf("expected at least one secret for SSO RBAC service account: %s", serviceAccount.GetName())
}
secret, err := s.cache.SecretLister.Secrets(serviceAccount.GetNamespace()).Get(serviceAccount.Secrets[0].Name)
secret, err := s.cache.GetSecret(serviceAccount.GetNamespace(), serviceAccount.Secrets[0].Name)
if err != nil {
return "", fmt.Errorf("failed to get service account secret: %w", err)
}
Expand Down
13 changes: 11 additions & 2 deletions server/cache/cache.go
View file
Open in desktop
Expand Up @@ -2,6 +2,8 @@ package cache

import (
"context"
v12 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"time"

"k8s.io/client-go/informers"
Expand All @@ -10,17 +12,24 @@ import (
)

type ResourceCache struct {
ctx context.Context
client kubernetes.Interface
v1.ServiceAccountLister
v1.SecretLister
}

func NewResourceCache(client kubernetes.Interface, ctx context.Context, namespace string) *ResourceCache {
informerFactory := informers.NewSharedInformerFactoryWithOptions(client, time.Minute*20, informers.WithNamespace(namespace))
cache := &ResourceCache{
ctx: ctx,
client: client,
ServiceAccountLister: informerFactory.Core().V1().ServiceAccounts().Lister(),
SecretLister: informerFactory.Core().V1().Secrets().Lister(),
}
informerFactory.Start(ctx.Done())
informerFactory.WaitForCacheSync(ctx.Done())
return cache
}

func (c *ResourceCache) GetSecret(namespace string, secretName string) (*v12.Secret, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason to introduce this method is if we want to introduce caching in future, we can easily change its implementation and keep external contract same

options := metav1.GetOptions{}
return c.client.CoreV1().Secrets(namespace).Get(c.ctx, secretName, options)
}
4 changes: 2 additions & 2 deletions server/cache/cache_test.go
View file
Open in desktop
Expand Up @@ -87,7 +87,7 @@ func TestServer_K8sUtilsCache(t *testing.T) {
assert.Equal(t, 1, len(sa))
assert.True(t, checkServiceAccountExists(sa, "sa3"))

secrets, _ := cache.SecretLister.Secrets("ns1").List(labels.Everything())
assert.Equal(t, 1, len(secrets))
secret, _ := cache.GetSecret("ns1", "s1")
assert.NotNil(t, secret)
})
}