From e20ef02a6af1ec8fc400a926959cf6c19cce3bcc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 27 May 2022 16:00:04 +0800 Subject: [PATCH] [Backport release-1.4] Feat: enhance controller auth by removing useless features & add authentication for componentrevision+healthcheck (#4023) * Feat: use application identity in gc & componentrevision & collectHealthStatus Signed-off-by: Somefive (cherry picked from commit 63fc4bcc693c2b6889508c50d915e9d44dfe94f9) * Chore: remove useless features and roles Signed-off-by: Somefive (cherry picked from commit f4ef77b2b318f6f193289146b21cc33236a523d4) * Fix: remove DELETE from mutating webhook Signed-off-by: Somefive (cherry picked from commit 75f3d5dc350314cd5978a24be0ce9e36ef4bf3e5) * Chore: enhance deploy error display Signed-off-by: Somefive (cherry picked from commit e69079bdaebddb2cb9124acfced215f531db3efb) * Fix: e2e test vela cli output match & controllerrevision recycle for serviceaccount impersonation Signed-off-by: Somefive (cherry picked from commit 05b85573a281bdc484f0e5c895ed55d63d483844) Co-authored-by: Somefive --- .../core.oam.dev/v1alpha1/envbinding_types.go | 3 +++ apis/types/types.go | 5 ---- .../cluster-gateway/cluster-gateway.yaml | 9 +++----- .../templates/kubevela-controller.yaml | 3 --- .../templates/rbac/x-definition.yaml | 23 ------------------- .../templates/cluster-gateway.yaml | 10 ++++---- .../templates/kubevela-controller.yaml | 3 --- cmd/core/main.go | 11 --------- e2e/application/application_test.go | 3 ++- pkg/auth/round_trippers.go | 6 +---- pkg/auth/round_trippers_test.go | 9 ++++---- .../v1alpha2/application/generator.go | 5 ++-- .../v1alpha2/application/revision.go | 5 ++-- pkg/features/controller_features.go | 3 --- pkg/resourcekeeper/componentrevision.go | 5 ++-- pkg/resourcekeeper/gc.go | 12 ++++++---- .../v1alpha2/application/mutating_handler.go | 11 ++++----- .../application/mutating_handler_test.go | 6 ++--- pkg/workflow/providers/multicluster/deploy.go | 2 +- .../multicluster_test.go | 8 ++++--- test/e2e-test/application_test.go | 8 ++++++- 21 files changed, 54 insertions(+), 96 deletions(-) delete mode 100644 charts/vela-core/templates/rbac/x-definition.yaml diff --git a/apis/core.oam.dev/v1alpha1/envbinding_types.go b/apis/core.oam.dev/v1alpha1/envbinding_types.go index 2313017cf..33858ce99 100644 --- a/apis/core.oam.dev/v1alpha1/envbinding_types.go +++ b/apis/core.oam.dev/v1alpha1/envbinding_types.go @@ -117,6 +117,9 @@ type PlacementDecision struct { // String encode placement decision func (in PlacementDecision) String() string { + if in.Namespace == "" { + return in.Cluster + } return in.Cluster + "/" + in.Namespace } diff --git a/apis/types/types.go b/apis/types/types.go index faf8c339b..ba476f387 100644 --- a/apis/types/types.go +++ b/apis/types/types.go @@ -169,8 +169,3 @@ const ( // VelaCoreConfig is to mark application, config and its secret or Terraform provider lelong to a KubeVela config VelaCoreConfig = "velacore-config" ) - -const ( - // ClusterGatewayAccessorGroup the group to impersonate which allows the access to the cluster-gateway - ClusterGatewayAccessorGroup = "cluster-gateway-accessor" -) diff --git a/charts/vela-core/templates/cluster-gateway/cluster-gateway.yaml b/charts/vela-core/templates/cluster-gateway/cluster-gateway.yaml index d624bc845..b7af88adc 100644 --- a/charts/vela-core/templates/cluster-gateway/cluster-gateway.yaml +++ b/charts/vela-core/templates/cluster-gateway/cluster-gateway.yaml @@ -129,7 +129,7 @@ spec: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-role + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy rules: - apiGroups: [ "cluster.core.oam.dev" ] resources: [ "clustergateways/proxy" ] @@ -138,15 +138,12 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-rolebinding + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-role + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy subjects: - - kind: Group - name: cluster-gateway-accessor - apiGroup: rbac.authorization.k8s.io - kind: Group name: kubevela:client apiGroup: rbac.authorization.k8s.io diff --git a/charts/vela-core/templates/kubevela-controller.yaml b/charts/vela-core/templates/kubevela-controller.yaml index ce81b03ec..ba94201ea 100644 --- a/charts/vela-core/templates/kubevela-controller.yaml +++ b/charts/vela-core/templates/kubevela-controller.yaml @@ -25,9 +25,6 @@ subjects: - kind: ServiceAccount name: {{ include "kubevela.serviceAccountName" . }} namespace: {{ .Release.Namespace }} - - kind: Group - name: core.oam.dev - apiGroup: rbac.authorization.k8s.io --- # permissions to do leader election. diff --git a/charts/vela-core/templates/rbac/x-definition.yaml b/charts/vela-core/templates/rbac/x-definition.yaml deleted file mode 100644 index ebf204418..000000000 --- a/charts/vela-core/templates/rbac/x-definition.yaml +++ /dev/null @@ -1,23 +0,0 @@ -{{ if .Values.authentication.enabled }} -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: kubevela:x-definition:reader -rules: - - apiGroups: [ "core.oam.dev" ] - resources: [ "componentdefinitions", "traitdefinitions", "workloaddefinitions", "workflowstepdefinitions", "policydefinitions", "definitionrevisions" ] - verbs: [ "get", "list", "watch" ] ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: kubevela:x-definition:reader-binding -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: kubevela:x-definition:reader -subjects: - - kind: Group - name: kubevela:x-definition:reader - apiGroup: rbac.authorization.k8s.io -{{ end }} \ No newline at end of file diff --git a/charts/vela-minimal/templates/cluster-gateway.yaml b/charts/vela-minimal/templates/cluster-gateway.yaml index dfdcbd3db..ec7ffcc2f 100644 --- a/charts/vela-minimal/templates/cluster-gateway.yaml +++ b/charts/vela-minimal/templates/cluster-gateway.yaml @@ -194,24 +194,22 @@ spec: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-role + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy rules: - apiGroups: [ "cluster.core.oam.dev" ] resources: [ "clustergateways/proxy" ] verbs: [ "get", "list", "watch", "create", "update", "patch", "delete" ] -{{ end }} --- -{{ if and .Values.multicluster.enabled }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-rolebinding + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: {{ include "kubevela.fullname" . }}:cluster-gateway-access-role + name: {{ include "kubevela.fullname" . }}:cluster-gateway:proxy subjects: - kind: Group - name: cluster-gateway-accessor + name: kubevela:client apiGroup: rbac.authorization.k8s.io {{ end }} \ No newline at end of file diff --git a/charts/vela-minimal/templates/kubevela-controller.yaml b/charts/vela-minimal/templates/kubevela-controller.yaml index 237c53d6e..ebea018f1 100644 --- a/charts/vela-minimal/templates/kubevela-controller.yaml +++ b/charts/vela-minimal/templates/kubevela-controller.yaml @@ -27,9 +27,6 @@ subjects: - kind: ServiceAccount name: {{ include "kubevela.serviceAccountName" . }} namespace: {{ .Release.Namespace }} - - kind: Group - name: core.oam.dev - apiGroup: rbac.authorization.k8s.io --- # permissions to do leader election. diff --git a/cmd/core/main.go b/cmd/core/main.go index c205d30fb..bee60cc9b 100644 --- a/cmd/core/main.go +++ b/cmd/core/main.go @@ -36,7 +36,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - apicommon "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/auth" ctrlClient "github.com/oam-dev/kubevela/pkg/client" @@ -46,13 +45,11 @@ import ( oamv1alpha2 "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2" "github.com/oam-dev/kubevela/pkg/controller/utils" "github.com/oam-dev/kubevela/pkg/cue/packages" - "github.com/oam-dev/kubevela/pkg/features" _ "github.com/oam-dev/kubevela/pkg/monitor/metrics" "github.com/oam-dev/kubevela/pkg/multicluster" "github.com/oam-dev/kubevela/pkg/oam" "github.com/oam-dev/kubevela/pkg/oam/discoverymapper" "github.com/oam-dev/kubevela/pkg/resourcekeeper" - pkgutils "github.com/oam-dev/kubevela/pkg/utils" "github.com/oam-dev/kubevela/pkg/utils/common" "github.com/oam-dev/kubevela/pkg/utils/system" "github.com/oam-dev/kubevela/pkg/utils/util" @@ -205,18 +202,10 @@ func main() { restConfig.QPS = float32(qps) restConfig.Burst = burst restConfig.Wrap(auth.NewImpersonatingRoundTripper) - if utilfeature.DefaultMutableFeatureGate.Enabled(features.ControllerAutoImpersonation) { - restConfig.Impersonate.UserName = types.VelaCoreName - restConfig.Impersonate.Groups = []string{apicommon.Group} - pkgutils.AutoSetSelfImpersonationInConfig(restConfig) - } klog.InfoS("Kubernetes Config Loaded", "UserAgent", restConfig.UserAgent, "QPS", restConfig.QPS, "Burst", restConfig.Burst, - "Auto-Impersonation", utilfeature.DefaultMutableFeatureGate.Enabled(features.ControllerAutoImpersonation), - "Impersonate-User", restConfig.Impersonate.UserName, - "Impersonate-Group", strings.Join(restConfig.Impersonate.Groups, ","), ) // wrapper the round tripper by multi cluster rewriter diff --git a/e2e/application/application_test.go b/e2e/application/application_test.go index c7f4a68f9..8ce90b9b7 100644 --- a/e2e/application/application_test.go +++ b/e2e/application/application_test.go @@ -19,6 +19,7 @@ package e2e import ( context2 "context" "fmt" + "strings" "time" "github.com/Netflix/go-expect" @@ -98,7 +99,7 @@ var ApplicationStatusDeeplyContext = func(context string, applicationName, workl cli := fmt.Sprintf("vela status %s", applicationName) output, err := e2e.LongTimeExec(cli, 120*time.Second) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - gomega.Expect(output).To(gomega.ContainSubstring("healthy")) + gomega.Expect(strings.ToLower(output)).To(gomega.ContainSubstring("healthy")) // TODO(zzxwill) need to check workloadType after app status is refined }) }) diff --git a/pkg/auth/round_trippers.go b/pkg/auth/round_trippers.go index ca2bd283b..f012d9d45 100644 --- a/pkg/auth/round_trippers.go +++ b/pkg/auth/round_trippers.go @@ -25,7 +25,6 @@ import ( "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/client-go/transport" - "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/utils" ) @@ -54,11 +53,8 @@ func (rt *impersonatingRoundTripper) RoundTrip(req *http.Request) (*http.Respons if exists && userInfo != nil { if name := userInfo.GetName(); name != "" { req.Header.Set(transport.ImpersonateUserHeader, name) - req.Header.Set(transport.ImpersonateGroupHeader, types.ClusterGatewayAccessorGroup) for _, group := range userInfo.GetGroups() { - if group != types.ClusterGatewayAccessorGroup { - req.Header.Add(transport.ImpersonateGroupHeader, group) - } + req.Header.Add(transport.ImpersonateGroupHeader, group) } q := req.URL.Query() q.Add(impersonateKey, "true") diff --git a/pkg/auth/round_trippers_test.go b/pkg/auth/round_trippers_test.go index 080e5ec46..999c0b6a1 100644 --- a/pkg/auth/round_trippers_test.go +++ b/pkg/auth/round_trippers_test.go @@ -31,7 +31,6 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/features" "github.com/oam-dev/kubevela/pkg/oam" ) @@ -66,21 +65,21 @@ func TestImpersonatingRoundTripper(t *testing.T) { return ContextWithUserInfo(ctx, app) }, expectedUser: "system:serviceaccount:vela-system:default", - expectedGroup: []string{types.ClusterGatewayAccessorGroup}, + expectedGroup: nil, }, "without service account and app": { ctxFn: func(ctx context.Context) context.Context { return ContextWithUserInfo(ctx, nil) }, expectedUser: "", - expectedGroup: []string{types.ClusterGatewayAccessorGroup}, + expectedGroup: nil, }, "without service account": { ctxFn: func(ctx context.Context) context.Context { return ContextWithUserInfo(ctx, &v1beta1.Application{}) }, expectedUser: AuthenticationDefaultUser, - expectedGroup: []string{types.ClusterGatewayAccessorGroup}, + expectedGroup: nil, }, "with user and groups": { ctxFn: func(ctx context.Context) context.Context { @@ -92,7 +91,7 @@ func TestImpersonatingRoundTripper(t *testing.T) { return ContextWithUserInfo(ctx, app) }, expectedUser: "username", - expectedGroup: []string{types.ClusterGatewayAccessorGroup, "kubevela:group1", "kubevela:group2"}, + expectedGroup: []string{"kubevela:group1", "kubevela:group2"}, }, } for name, ts := range testSets { diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go index 00e21ae59..74ae1e524 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go @@ -30,6 +30,7 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/appfile" + "github.com/oam-dev/kubevela/pkg/auth" "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/assemble" "github.com/oam-dev/kubevela/pkg/cue/model/value" "github.com/oam-dev/kubevela/pkg/cue/process" @@ -219,7 +220,7 @@ func (h *AppHandler) checkComponentHealth(appParser *appfile.Parser, appRev *v1b if err != nil { return false, err } - wl.Ctx.SetCtx(ctx) + wl.Ctx.SetCtx(auth.ContextWithUserInfo(ctx, h.app)) readyWorkload, readyTraits, err := renderComponentsAndTraits(h.r.Client, manifest, appRev, clusterName, overrideNamespace, env) if err != nil { @@ -258,7 +259,7 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet return nil, nil, false, errors.WithMessage(err, "cannot dispatch packaged workload resources") } } - wl.Ctx.SetCtx(ctx) + wl.Ctx.SetCtx(auth.ContextWithUserInfo(ctx, h.app)) readyWorkload, readyTraits, err := renderComponentsAndTraits(h.r.Client, manifest, appRev, clusterName, overrideNamespace, env) if err != nil { diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index 910c1ba36..98d9b225a 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -42,6 +42,7 @@ import ( "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/appfile" helmapi "github.com/oam-dev/kubevela/pkg/appfile/helm/flux2apis" + "github.com/oam-dev/kubevela/pkg/auth" "github.com/oam-dev/kubevela/pkg/component" "github.com/oam-dev/kubevela/pkg/controller/utils" "github.com/oam-dev/kubevela/pkg/cue/model" @@ -542,7 +543,7 @@ func (h *AppHandler) handleComponentRevisionNameSpecified(ctx context.Context, c revisionName := comp.ExternalRevision cr := &appsv1.ControllerRevision{} - if err := h.r.Client.Get(ctx, client.ObjectKey{Namespace: h.getComponentRevisionNamespace(ctx), Name: revisionName}, cr); err != nil { + if err := h.r.Client.Get(auth.ContextWithUserInfo(ctx, h.app), client.ObjectKey{Namespace: h.getComponentRevisionNamespace(ctx), Name: revisionName}, cr); err != nil { if !apierrors.IsNotFound(err) { return errors.Wrapf(err, "failed to get controllerRevision:%s", revisionName) } @@ -592,7 +593,7 @@ func (h *AppHandler) handleComponentRevisionNameUnspecified(ctx context.Context, listOpts := []client.ListOption{client.MatchingLabels{ oam.LabelControllerRevisionComponent: comp.Name, }, client.InNamespace(h.getComponentRevisionNamespace(ctx))} - if err := h.r.List(ctx, crList, listOpts...); err != nil { + if err := h.r.List(auth.ContextWithUserInfo(ctx, h.app), crList, listOpts...); err != nil { return err } diff --git a/pkg/features/controller_features.go b/pkg/features/controller_features.go index 05fc28f3b..eec989cea 100644 --- a/pkg/features/controller_features.go +++ b/pkg/features/controller_features.go @@ -36,8 +36,6 @@ const ( // Edge Features - // ControllerAutoImpersonation enable the auto impersonation for controller (to use explicit identity for requests) - ControllerAutoImpersonation featuregate.Feature = "ControllerAutoImpersonation" // AuthenticateApplication enable the authentication for application AuthenticateApplication featuregate.Feature = "AuthenticateApplication" ) @@ -47,7 +45,6 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ LegacyObjectTypeIdentifier: {Default: false, PreRelease: featuregate.Alpha}, DeprecatedObjectLabelSelector: {Default: false, PreRelease: featuregate.Alpha}, LegacyResourceTrackerGC: {Default: true, PreRelease: featuregate.Alpha}, - ControllerAutoImpersonation: {Default: true, PreRelease: featuregate.Alpha}, AuthenticateApplication: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/pkg/resourcekeeper/componentrevision.go b/pkg/resourcekeeper/componentrevision.go index 6afef7d1e..396fb51f1 100644 --- a/pkg/resourcekeeper/componentrevision.go +++ b/pkg/resourcekeeper/componentrevision.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/pkg/auth" "github.com/oam-dev/kubevela/pkg/multicluster" "github.com/oam-dev/kubevela/pkg/oam" "github.com/oam-dev/kubevela/pkg/resourcetracker" @@ -45,7 +46,7 @@ func (h *resourceKeeper) DispatchComponentRevision(ctx context.Context, cr *v1.C if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, []*unstructured.Unstructured{obj}, true, common.WorkflowResourceCreator); err != nil { return errors.Wrapf(err, "failed to record componentrevision %s/%s/%s", oam.GetCluster(cr), cr.Namespace, cr.Name) } - if err = h.Client.Create(multicluster.ContextWithClusterName(ctx, oam.GetCluster(cr)), cr); err != nil { + if err = h.Client.Create(auth.ContextWithUserInfo(multicluster.ContextWithClusterName(ctx, oam.GetCluster(cr)), h.app), cr); err != nil { return errors.Wrapf(err, "failed to create componentrevision %s/%s/%s", oam.GetCluster(cr), cr.Namespace, cr.Name) } return nil @@ -63,7 +64,7 @@ func (h *resourceKeeper) DeleteComponentRevision(ctx context.Context, cr *v1.Con obj.SetName(cr.Name) obj.SetNamespace(cr.Namespace) obj.SetLabels(cr.Labels) - if err = h.Client.Delete(multicluster.ContextWithClusterName(ctx, oam.GetCluster(cr)), cr); err != nil && !errors2.IsNotFound(err) { + if err = h.Client.Delete(auth.ContextWithUserInfo(multicluster.ContextWithClusterName(ctx, oam.GetCluster(cr)), h.app), cr); err != nil && !errors2.IsNotFound(err) { return errors.Wrapf(err, "failed to delete componentrevision %s/%s/%s", oam.GetCluster(cr), cr.Namespace, cr.Name) } if err = resourcetracker.DeletedManifestInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, obj, true); err != nil { diff --git a/pkg/resourcekeeper/gc.go b/pkg/resourcekeeper/gc.go index 980db5d76..4a2d0c219 100644 --- a/pkg/resourcekeeper/gc.go +++ b/pkg/resourcekeeper/gc.go @@ -35,6 +35,7 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha1" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/auth" "github.com/oam-dev/kubevela/pkg/features" "github.com/oam-dev/kubevela/pkg/monitor/metrics" "github.com/oam-dev/kubevela/pkg/multicluster" @@ -182,7 +183,7 @@ func (h *gcHandler) scan(ctx context.Context) (inactiveRTs []*v1beta1.ResourceTr if rt != nil { inactive := true for _, mr := range rt.Spec.ManagedResources { - entry := h.cache.get(ctx, mr) + entry := h.cache.get(auth.ContextWithUserInfo(ctx, h.app), mr) if entry.err == nil && (entry.gcExecutorRT != rt || !entry.exists) { continue } @@ -225,7 +226,7 @@ func (h *gcHandler) Mark(ctx context.Context) error { // checkAndRemoveResourceTrackerFinalizer return (all resource recycled, error) func (h *gcHandler) checkAndRemoveResourceTrackerFinalizer(ctx context.Context, rt *v1beta1.ResourceTracker) (bool, v1beta1.ManagedResource, error) { for _, mr := range rt.Spec.ManagedResources { - entry := h.cache.get(ctx, mr) + entry := h.cache.get(auth.ContextWithUserInfo(ctx, h.app), mr) if entry.err != nil { return false, entry.mr, entry.err } @@ -257,6 +258,7 @@ func (h *gcHandler) Sweep(ctx context.Context) (finished bool, waiting []v1beta1 } func (h *gcHandler) recycleResourceTracker(ctx context.Context, rt *v1beta1.ResourceTracker) error { + ctx = auth.ContextWithUserInfo(ctx, h.app) switch h.cfg.order { case v1alpha1.OrderDependency: for _, mr := range rt.Spec.ManagedResources { @@ -380,14 +382,16 @@ func (h *gcHandler) GarbageCollectComponentRevisionResourceTracker(ctx context.C } var managedResources []v1beta1.ManagedResource for _, cr := range h._crRT.Spec.ManagedResources { // legacy code for rollout-plan + _ctx := multicluster.ContextWithClusterName(ctx, cr.Cluster) + _ctx = auth.ContextWithUserInfo(_ctx, h.app) if _, exists := inUseComponents[cr.ComponentKey()]; !exists { _cr := &appsv1.ControllerRevision{} - err := h.Client.Get(multicluster.ContextWithClusterName(ctx, cr.Cluster), cr.NamespacedName(), _cr) + err := h.Client.Get(_ctx, cr.NamespacedName(), _cr) if err != nil && !multicluster.IsNotFoundOrClusterNotExists(err) { return errors.Wrapf(err, "failed to get component revision %s", cr.ResourceKey()) } if err == nil { - if err = h.Client.Delete(multicluster.ContextWithClusterName(ctx, cr.Cluster), _cr); err != nil && !kerrors.IsNotFound(err) { + if err = h.Client.Delete(_ctx, _cr); err != nil && !kerrors.IsNotFound(err) { return errors.Wrapf(err, "failed to delete component revision %s", cr.ResourceKey()) } } diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler.go b/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler.go index 3e5d2c8f5..483777a12 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler.go +++ b/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler.go @@ -30,7 +30,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/pkg/auth" "github.com/oam-dev/kubevela/pkg/features" @@ -52,7 +51,7 @@ func (h *MutatingHandler) Handle(ctx context.Context, req admission.Request) adm return admission.Patched("") } - if slices.Contains(req.UserInfo.Groups, common.Group) || slices.Contains(h.skipUsers, req.UserInfo.Username) { + if slices.Contains(h.skipUsers, req.UserInfo.Username) { return admission.Patched("") } @@ -86,11 +85,9 @@ func (h *MutatingHandler) InjectDecoder(d *admission.Decoder) error { func RegisterMutatingHandler(mgr manager.Manager) { server := mgr.GetWebhookServer() handler := &MutatingHandler{} - if !utilfeature.DefaultMutableFeatureGate.Enabled(features.ControllerAutoImpersonation) { - if userInfo := utils.GetUserInfoFromConfig(mgr.GetConfig()); userInfo != nil { - klog.Infof("[ApplicationMutatingHandler] add skip user %s", userInfo.Username) - handler.skipUsers = []string{userInfo.Username} - } + if userInfo := utils.GetUserInfoFromConfig(mgr.GetConfig()); userInfo != nil { + klog.Infof("[ApplicationMutatingHandler] add skip user %s", userInfo.Username) + handler.skipUsers = []string{userInfo.Username} } server.Register("/mutating-core-oam-dev-v1beta1-applications", &webhook.Admission{Handler: handler}) } diff --git a/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler_test.go b/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler_test.go index 2778e6ef5..a31027d59 100644 --- a/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler_test.go +++ b/pkg/webhook/core.oam.dev/v1alpha2/application/mutating_handler_test.go @@ -29,8 +29,8 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/features" "github.com/oam-dev/kubevela/pkg/oam" ) @@ -40,7 +40,7 @@ var _ = Describe("Test Application Mutator", func() { var mutatingHandler *MutatingHandler BeforeEach(func() { - mutatingHandler = &MutatingHandler{} + mutatingHandler = &MutatingHandler{skipUsers: []string{types.VelaCoreName}} Expect(mutatingHandler.InjectDecoder(decoder)).Should(BeNil()) }) @@ -55,7 +55,7 @@ var _ = Describe("Test Application Mutator", func() { Expect(utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=true", features.AuthenticateApplication))).Should(Succeed()) resp := mutatingHandler.Handle(ctx, admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ - UserInfo: authv1.UserInfo{Groups: []string{common.Group}}, + UserInfo: authv1.UserInfo{Username: types.VelaCoreName}, }}) Expect(resp.Allowed).Should(BeTrue()) Expect(resp.Patches).Should(BeNil()) diff --git a/pkg/workflow/providers/multicluster/deploy.go b/pkg/workflow/providers/multicluster/deploy.go index d937db6c8..4c7697247 100644 --- a/pkg/workflow/providers/multicluster/deploy.go +++ b/pkg/workflow/providers/multicluster/deploy.go @@ -208,7 +208,7 @@ func applyComponents(apply oamProvider.ComponentApply, healthCheck oamProvider.C var reasons []string for i, res := range results { if res.err != nil { - errs = append(errs, res.err) + errs = append(errs, fmt.Errorf("error encountered in cluster %s: %w", todoTasks[i].placement.Cluster, res.err)) } if !res.healthy { allHealthy = false diff --git a/test/e2e-multicluster-test/multicluster_test.go b/test/e2e-multicluster-test/multicluster_test.go index 26399af02..27608f02b 100644 --- a/test/e2e-multicluster-test/multicluster_test.go +++ b/test/e2e-multicluster-test/multicluster_test.go @@ -328,9 +328,11 @@ var _ = Describe("Test multicluster scenario", func() { envs[0].Placement.ClusterSelector.Name = WorkerClusterName bs, err = json.Marshal(&v1alpha1.EnvBindingSpec{Envs: []v1alpha1.EnvConfig{envs[0]}}) Expect(err).Should(Succeed()) - Expect(k8sClient.Get(hubCtx, namespacedName, app)).Should(Succeed()) - app.Spec.Policies[0].Properties.Raw = bs - Expect(k8sClient.Update(hubCtx, app)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, namespacedName, app)).Should(Succeed()) + app.Spec.Policies[0].Properties.Raw = bs + g.Expect(k8sClient.Update(hubCtx, app)).Should(Succeed()) + }, 15*time.Second).Should(Succeed()) Eventually(func(g Gomega) { deploys := &appsv1.DeploymentList{} g.Expect(k8sClient.List(hubCtx, deploys, client.InNamespace(testNamespace))).Should(Succeed()) diff --git a/test/e2e-test/application_test.go b/test/e2e-test/application_test.go index 771bc205e..a1b7efdfe 100644 --- a/test/e2e-test/application_test.go +++ b/test/e2e-test/application_test.go @@ -28,6 +28,7 @@ import ( v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -364,7 +365,7 @@ var _ = Describe("Application Normal tests", func() { { Verbs: []string{rbacv1.VerbAll}, APIGroups: []string{"apps"}, - Resources: []string{"deployments"}, + Resources: []string{"deployments", "controllerrevisions"}, }, }, } @@ -402,6 +403,11 @@ var _ = Describe("Application Normal tests", func() { By("Checking an application status") verifyWorkloadRunningExpected("myweb", 1, "stefanprodan/podinfo:4.0.3") verifyComponentRevision("myweb", 1) + + Expect(k8sClient.Delete(ctx, &newApp)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&newApp), &newApp)).Should(Satisfy(errors.IsNotFound)) + }, 15*time.Second).Should(Succeed()) }) It("Test app with ServiceAccount which has no permission for the component", func() {