From 979911d541029be214723eea57e2dc9d241953e3 Mon Sep 17 00:00:00 2001 From: Yijie Wang Date: Tue, 2 Jan 2024 16:05:13 -0500 Subject: [PATCH] Revert "Merge remote-tracking branch 'otherfork/main' into main" This reverts commit 94cd51bdbdbb026b1c3ec2b004e0e4dfd564ea19, reversing changes made to 0bf29828a296f478e47d2d3c9a992372050f15cf. --- docs/eventing-api.md | 10 -- pkg/apis/sources/v1/ping_types.go | 2 - pkg/apis/sources/v1/zz_generated.deepcopy.go | 5 - pkg/reconciler/pingsource/pingsource.go | 102 +--------------- pkg/reconciler/pingsource/pingsource_test.go | 14 --- .../pingsource/resources/oidc_rolebinding.go | 115 ------------------ 6 files changed, 2 insertions(+), 246 deletions(-) delete mode 100644 pkg/reconciler/pingsource/resources/oidc_rolebinding.go diff --git a/docs/eventing-api.md b/docs/eventing-api.md index be83e05f830..b48faf9b2c3 100644 --- a/docs/eventing-api.md +++ b/docs/eventing-api.md @@ -6232,16 +6232,6 @@ state. Source.

- - -namespaces
- -[]string - - - - -

SinkBindingSpec diff --git a/pkg/apis/sources/v1/ping_types.go b/pkg/apis/sources/v1/ping_types.go index b752da20442..5390fc288ff 100644 --- a/pkg/apis/sources/v1/ping_types.go +++ b/pkg/apis/sources/v1/ping_types.go @@ -93,8 +93,6 @@ type PingSourceStatus struct { // * SinkURI - the current active sink URI that has been configured for the // Source. duckv1.SourceStatus `json:",inline"` - - Namespaces []string `json:"namespaces"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/sources/v1/zz_generated.deepcopy.go b/pkg/apis/sources/v1/zz_generated.deepcopy.go index 3bed1735e4a..6d175e3c960 100644 --- a/pkg/apis/sources/v1/zz_generated.deepcopy.go +++ b/pkg/apis/sources/v1/zz_generated.deepcopy.go @@ -358,11 +358,6 @@ func (in *PingSourceSpec) DeepCopy() *PingSourceSpec { func (in *PingSourceStatus) DeepCopyInto(out *PingSourceStatus) { *out = *in in.SourceStatus.DeepCopyInto(&out.SourceStatus) - if in.Namespaces != nil { - in, out := &in.Namespaces, &out.Namespaces - *out = make([]string, len(*in)) - copy(*out, *in) - } return } diff --git a/pkg/reconciler/pingsource/pingsource.go b/pkg/reconciler/pingsource/pingsource.go index 2297ff4ad5e..cd88c938646 100644 --- a/pkg/reconciler/pingsource/pingsource.go +++ b/pkg/reconciler/pingsource/pingsource.go @@ -21,7 +21,7 @@ import ( "encoding/json" "fmt" - clientv1 "k8s.io/client-go/listers/core/v1" + v1 "k8s.io/client-go/listers/core/v1" "go.uber.org/zap" @@ -41,7 +41,6 @@ import ( "knative.dev/pkg/system" "knative.dev/pkg/tracker" - rbacv1listers "k8s.io/client-go/listers/rbac/v1" "knative.dev/eventing/pkg/adapter/mtping" "knative.dev/eventing/pkg/adapter/v2" "knative.dev/eventing/pkg/apis/feature" @@ -80,10 +79,7 @@ type Reconciler struct { // Leader election configuration for the mt receive adapter leConfig string - serviceAccountLister clientv1.ServiceAccountLister - roleLister rbacv1listers.RoleLister - roleBindingLister rbacv1listers.RoleBindingLister - namespaceLister clientv1.NamespaceLister + serviceAccountLister v1.ServiceAccountLister } // Check that our Reconciler implements ReconcileKind @@ -117,23 +113,6 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, source *sourcesv1.PingSo return err } - if featureFlags.IsOIDCAuthentication() { - // Create the role - err := r.createOIDCRole(ctx, source) - - if err != nil { - logging.FromContext(ctx).Errorw("Failed when creating the OIDC Role for PingSource", zap.Error(err)) - return err - } - - // Create the rolebinding - err = r.createOIDCRoleBinding(ctx, source) - if err != nil { - logging.FromContext(ctx).Errorw("Failed when creating the OIDC RoleBinding for PingSource", zap.Error(err)) - return err - } - } - sinkAddr, err := r.sinkResolver.AddressableFromDestinationV1(ctx, *dest, source) if err != nil { source.Status.MarkNoSink("NotFound", "") @@ -235,80 +214,3 @@ func findContainer(podSpec *corev1.PodSpec, name string) *corev1.Container { func zero(i *int32) bool { return i != nil && *i == 0 } - -func (r *Reconciler) createOIDCRole(ctx context.Context, source *sourcesv1.PingSource) error { - roleName := resources.GetOIDCTokenRoleName(source.Name) - - expected, err := resources.MakeOIDCRole(source) - - if err != nil { - return fmt.Errorf("Cannot create OIDC role for PingSource %s/%s: %w", source.GetName(), source.GetNamespace(), err) - } - // By querying roleLister to see whether the role exist or not - role, err := r.roleLister.Roles(source.GetNamespace()).Get(roleName) - - if apierrors.IsNotFound(err) { - // If the role does not exist, we will call kubeclient to create it - role = expected - _, err = r.kubeClientSet.RbacV1().Roles(source.GetNamespace()).Create(ctx, role, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("could not create OIDC service account role %s/%s for %s: %w", source.GetName(), source.GetNamespace(), "ApiServerSource", err) - } - } else { - // If the role does exist, we will check whether an update is needed - // By comparing the role's rule - if !equality.Semantic.DeepEqual(role.Rules, expected.Rules) { - // If the role's rules are not equal, we will update the role - role.Rules = expected.Rules - _, err = r.kubeClientSet.RbacV1().Roles(source.GetNamespace()).Update(ctx, role, metav1.UpdateOptions{}) - if err != nil { - return fmt.Errorf("could not update OIDC service account role %s/%s for %s: %w", source.GetName(), source.GetNamespace(), "ApiServerSource", err) - } - } else { - // If the role does exist and no update is needed, we will just return - return nil - } - } - - return nil - -} - -// createOIDCRoleBinding: this function will call resources package to get the rolebinding object -// and then pass to kubeclient to make the actual OIDC rolebinding -func (r *Reconciler) createOIDCRoleBinding(ctx context.Context, source *sourcesv1.PingSource) error { - roleBindingName := resources.GetOIDCTokenRoleBindingName(source.Name) - - expected, err := resources.MakeOIDCRoleBinding(source) - if err != nil { - return fmt.Errorf("Cannot create OIDC roleBinding for PingSource %s/%s: %w", source.GetName(), source.GetNamespace(), err) - } - - // By querying roleBindingLister to see whether the roleBinding exist or not - roleBinding, err := r.roleBindingLister.RoleBindings(source.GetNamespace()).Get(roleBindingName) - if apierrors.IsNotFound(err) { - // If the role does not exist, we will call kubeclient to create it - roleBinding = expected - _, err = r.kubeClientSet.RbacV1().RoleBindings(source.GetNamespace()).Create(ctx, roleBinding, metav1.CreateOptions{}) - if err != nil { - return fmt.Errorf("could not create OIDC service account rolebinding %s/%s for %s: %w", source.GetName(), source.GetNamespace(), "apiserversource", err) - } - } else { - // If the role does exist, we will check whether an update is needed - // By comparing the role's rule - if !equality.Semantic.DeepEqual(roleBinding.RoleRef, expected.RoleRef) || !equality.Semantic.DeepEqual(roleBinding.Subjects, expected.Subjects) { - // If the role's rules are not equal, we will update the role - roleBinding.RoleRef = expected.RoleRef - roleBinding.Subjects = expected.Subjects - _, err = r.kubeClientSet.RbacV1().RoleBindings(source.GetNamespace()).Update(ctx, roleBinding, metav1.UpdateOptions{}) - if err != nil { - return fmt.Errorf("could not update OIDC service account rolebinding %s/%s for %s: %w", source.GetName(), source.GetNamespace(), "apiserversource", err) - } - } else { - // If the role does exist and no update is needed, we will just return - return nil - } - } - - return nil -} diff --git a/pkg/reconciler/pingsource/pingsource_test.go b/pkg/reconciler/pingsource/pingsource_test.go index 036082c9c4a..4e84a050926 100644 --- a/pkg/reconciler/pingsource/pingsource_test.go +++ b/pkg/reconciler/pingsource/pingsource_test.go @@ -87,20 +87,6 @@ var ( Name: &sinkURL.Scheme, URL: sinkURL, } - sinkAudience = "sink-oidc-audience" - sinkOIDCAddressable = &duckv1.Addressable{ - Name: &sinkURL.Scheme, - URL: sinkURL, - Audience: &sinkAudience, - } - sinkOIDCDest = duckv1.Destination{ - Ref: &duckv1.KReference{ - Name: sinkName, - Kind: "Channel", - APIVersion: "messaging.knative.dev/v1", - }, - Audience: &sinkAudience, - } ) const ( diff --git a/pkg/reconciler/pingsource/resources/oidc_rolebinding.go b/pkg/reconciler/pingsource/resources/oidc_rolebinding.go deleted file mode 100644 index 91e51600515..00000000000 --- a/pkg/reconciler/pingsource/resources/oidc_rolebinding.go +++ /dev/null @@ -1,115 +0,0 @@ -/* -Copyright 2020 The Knative Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// taken from #7452, with modifications for pingsource - -package resources - -import ( - "fmt" - - "knative.dev/eventing/pkg/apis/sources" - - "knative.dev/pkg/kmeta" - - rbacv1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - v1 "knative.dev/eventing/pkg/apis/sources/v1" -) - -// GetOIDCTokenRoleName will return the name of the role for creating the JWT token -func GetOIDCTokenRoleName(sourceName string) string { - return kmeta.ChildName(sourceName, "-create-oidc-token") -} - -// GetOIDCTokenRoleBindingName will return the name of the rolebinding for creating the JWT token -func GetOIDCTokenRoleBindingName(sourceName string) string { - return kmeta.ChildName(sourceName, "-create-oidc-token") -} - -func MakeOIDCRole(source *v1.PingSource) (*rbacv1.Role, error) { - roleName := GetOIDCTokenRoleName(source.Name) - - if source.Status.Auth == nil || source.Status.Auth.ServiceAccountName == nil { - return nil, fmt.Errorf("Error when making OIDC Role for pingsource, as the OIDC service account does not exist") - } - - return &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{ - Name: roleName, - Namespace: source.GetNamespace(), - Annotations: map[string]string{ - "description": fmt.Sprintf("Role for OIDC Authentication for PingSource %q", source.GetName()), - }, - Labels: map[string]string{ - sources.OIDCLabelKey: "", - }, - OwnerReferences: []metav1.OwnerReference{ - *kmeta.NewControllerRef(source), - }, - }, - Rules: []rbacv1.PolicyRule{ - rbacv1.PolicyRule{ - APIGroups: []string{""}, - // apiServerSource OIDC service account name, it is in the source.Status, NOT in source.Spec - ResourceNames: []string{*source.Status.Auth.ServiceAccountName}, - Resources: []string{"serviceaccounts/token"}, - Verbs: []string{"create"}, - }, - }, - }, nil - -} - -// MakeOIDCRoleBinding will return the rolebinding object for generating the JWT token -func MakeOIDCRoleBinding(source *v1.PingSource) (*rbacv1.RoleBinding, error) { - roleName := GetOIDCTokenRoleName(source.Name) - roleBindingName := GetOIDCTokenRoleBindingName(source.Name) - - if *source.Status.Auth.ServiceAccountName == "" { - return nil, fmt.Errorf("Error when making OIDC RoleBinding for pingserversource, as the Spec service account does not exist") - } - - return &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: roleBindingName, - Namespace: source.GetNamespace(), - Annotations: map[string]string{ - "description": fmt.Sprintf("Role Binding for OIDC Authentication for PingServerSource %q", source.GetName()), - }, - Labels: map[string]string{ - sources.OIDCLabelKey: "", - }, - OwnerReferences: []metav1.OwnerReference{ - *kmeta.NewControllerRef(source), - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: roleName, - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - Namespace: source.GetNamespace(), - //Note: apiServerSource service account name, it is in the source.Spec, NOT in source.Status.Auth - Name: *source.Status.Auth.ServiceAccountName, - }, - }, - }, nil - -}