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

Use filtered informer to watch OIDC service accounts #7527

Merged
merged 41 commits into from
Jan 30, 2024
Merged

Use filtered informer to watch OIDC service accounts #7527

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
0bf2982
controller.go changed
yijie-04 Dec 23, 2023
17c775a
#7320 WIP
Zazzscoot Dec 24, 2023
94cd51b
Merge remote-tracking branch 'otherfork/main' into main
yijie-04 Dec 24, 2023
a82c2aa
WIP: Testing filtered informer (knative#7341)
yijie-04 Dec 24, 2023
d4bfe4e
unit test passed
yijie-04 Jan 2, 2024
979911d
Revert "Merge remote-tracking branch 'otherfork/main' into main"
yijie-04 Jan 2, 2024
bbefcc2
Removed comments
yijie-04 Jan 6, 2024
de30fc5
Changed to filtered informer for Subscription identity service account
yijie-04 Jan 6, 2024
ce5a778
Changed to filtered informer for Sequence service accounts
yijie-04 Jan 6, 2024
2ae1090
Changed to filtered informer for Parallel identity service accounts
yijie-04 Jan 6, 2024
695e58c
Changed to filtered informer for APIServerSource identity service acc…
yijie-04 Jan 6, 2024
efc1cc3
fixed unit tests
yijie-04 Jan 10, 2024
37f0dff
Merge branch 'main' into main
yijie-04 Jan 10, 2024
a414f3e
added label selector for mtchannel_broker
yijie-04 Jan 10, 2024
de787c6
added filtered informer for sinkbinding identity service accounts
yijie-04 Jan 11, 2024
c365d4a
added OIDC label selector in webhook
yijie-04 Jan 12, 2024
7d73360
added filtered informer for containersource service accounts
yijie-04 Jan 12, 2024
c048610
added filtered informer for pingsource service accounts
yijie-04 Jan 12, 2024
3d3bd2c
added OIDC label selector in apiserver ctx
yijie-04 Jan 12, 2024
f5d583f
added OIDC label selector in broker/filter
yijie-04 Jan 12, 2024
d934231
added OIDC label selector in broker/ingress
yijie-04 Jan 12, 2024
08dbe1e
added OIDC label selector in in_memory/channel_dispatcher
yijie-04 Jan 12, 2024
d3205ab
added OIDC label selector in mtping
yijie-04 Jan 12, 2024
a31fc39
fixed unit test issues for pingsource
yijie-04 Jan 12, 2024
b69cc29
fixed unit test for container source
yijie-04 Jan 12, 2024
51befc7
Merge branch 'knative:main' into main
yijie-04 Jan 13, 2024
e95329e
formatted files
yijie-04 Jan 19, 2024
e76634a
Merge remote-tracking branch 'upstream/main' into main
yijie-04 Jan 19, 2024
8e112d8
updated service account informer in apiserversource
yijie-04 Jan 22, 2024
c04375a
updated service account informers in other places
yijie-04 Jan 22, 2024
d7f6e43
small typo fix
yijie-04 Jan 22, 2024
f2fe553
added actual value for OIDC label
yijie-04 Jan 24, 2024
b2941ac
added a valid value for OIDClabelkey
yijie-04 Jan 26, 2024
5244f19
Merge remote-tracking branch 'upstream/main' into main
yijie-04 Jan 26, 2024
2c94ec7
changed references of OIDCLabelKey
yijie-04 Jan 26, 2024
3d0d399
fixed import path problem
yijie-04 Jan 26, 2024
8c30426
changed OIDCLabelSelector in all main.go files
yijie-04 Jan 26, 2024
bee47f4
changed instances of OIDCLabelSelector in controller and controller t…
yijie-04 Jan 26, 2024
5c25449
deleted OIDC related labels from register.go
yijie-04 Jan 26, 2024
63346a6
fixed formatting issues
yijie-04 Jan 27, 2024
9a44a73
Added value for OIDCLabelKey
yijie-04 Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/auth/serviceaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"knative.dev/pkg/kmeta"
pkgreconciler "knative.dev/pkg/reconciler"

"knative.dev/eventing/pkg/apis/sources"

"go.uber.org/zap"
v1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -66,6 +68,9 @@ func GetOIDCServiceAccountForResource(gvk schema.GroupVersionKind, objectMeta me
Annotations: map[string]string{
"description": fmt.Sprintf("Service Account for OIDC Authentication for %s %q", gvk.GroupKind().Kind, objectMeta.Name),
},
Labels: map[string]string{
sources.OIDCLabelKey: "",
Copy link
Member

Choose a reason for hiding this comment

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

nit: we probably want to have an actual value for this label, rather than just an empty string.

Copy link
Contributor Author

@yijie-04 yijie-04 Jan 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@Cali0707 Thank you for your comments! Would something like "OIDC label" be a good value for this?

Copy link
Member

Choose a reason for hiding this comment

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

I am OK with using the same selector for the OIDC related resources (e.g. use it for the OIDC created role & rolebinding of the apiserversource), but could we then:

  • move it to a more general package as of source (e.g. auth?!?)
  • Rename the OIDCTokenRoleLabelSelector constant then to something more generic?

},
},
}
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/reconciler/broker/trigger/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ package mttrigger
import (
"context"

"knative.dev/eventing/pkg/apis/sources"

"go.uber.org/zap"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/tools/cache"
"knative.dev/pkg/client/injection/ducks/duck/v1/source"
configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap"
serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
//serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount"
yijie-04 marked this conversation as resolved.
Show resolved Hide resolved
"knative.dev/pkg/configmap"
"knative.dev/pkg/controller"
"knative.dev/pkg/injection/clients/dynamicclient"
Expand All @@ -45,6 +47,8 @@ import (
eventinglisters "knative.dev/eventing/pkg/client/listers/eventing/v1"
"knative.dev/eventing/pkg/duck"
kubeclient "knative.dev/pkg/client/injection/kube/client"

serviceaccountinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered"
)

// NewController initializes the controller and is called by the generated code
Expand All @@ -59,7 +63,7 @@ func NewController(
subscriptionInformer := subscriptioninformer.Get(ctx)
configmapInformer := configmapinformer.Get(ctx)
secretInformer := secretinformer.Get(ctx)
serviceaccountInformer := serviceaccountinformer.Get(ctx)
serviceaccountInformer := serviceaccountinformer.Get(ctx, sources.OIDCTokenRoleLabelSelector)

featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"))
featureStore.WatchConfigs(cmw)
Expand Down Expand Up @@ -113,7 +117,7 @@ func NewController(

// Reconciler Trigger when the OIDC service account changes
serviceaccountInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
FilterFunc: controller.FilterController(&eventing.Trigger{}),
FilterFunc: controller.FilterController(&eventing.Trigger{}), // replace with filtered informer
yijie-04 marked this conversation as resolved.
Show resolved Hide resolved
Handler: controller.HandleAll(impl.EnqueueControllerOf),
})

Expand Down
14 changes: 12 additions & 2 deletions pkg/reconciler/broker/trigger/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ limitations under the License.
package mttrigger

import (
"context"
"fmt"
"testing"

"knative.dev/eventing/pkg/apis/sources"
filteredFactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -41,14 +45,15 @@ import (
_ "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/secret/fake"

// Fake injection informers
_ "knative.dev/pkg/client/injection/kube/informers/factory/filtered/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/broker/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1/trigger/fake"
_ "knative.dev/eventing/pkg/client/injection/informers/messaging/v1/subscription/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/fake"
)

func TestNew(t *testing.T) {
ctx, _ := SetupFakeContext(t)
ctx, _ := SetupFakeContext(t, SetUpInformerSelector)

c := NewController(ctx, configmap.NewStaticWatcher(&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "config-features"}}))

Expand All @@ -57,6 +62,11 @@ func TestNew(t *testing.T) {
}
}

func SetUpInformerSelector(ctx context.Context) context.Context {
ctx = filteredFactory.WithSelectors(ctx, sources.OIDCTokenRoleLabelSelector)
return ctx
}

func TestFilterTriggers(t *testing.T) {
ctx, _ := SetupFakeContext(t)

Expand Down
52 changes: 52 additions & 0 deletions ...tive.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/fake/fake.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 65 additions & 0 deletions ...dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/serviceaccount.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,8 @@ knative.dev/pkg/client/injection/kube/informers/core/v1/service
knative.dev/pkg/client/injection/kube/informers/core/v1/service/fake
knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount
knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/fake
knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered
knative.dev/pkg/client/injection/kube/informers/core/v1/serviceaccount/filtered/fake
knative.dev/pkg/client/injection/kube/informers/factory
knative.dev/pkg/client/injection/kube/informers/factory/fake
knative.dev/pkg/client/injection/kube/informers/factory/filtered
Expand Down