From 2b4d12fbddca374beece47a4c34980e29cda79c2 Mon Sep 17 00:00:00 2001 From: chival <47812250+chivalryq@users.noreply.github.com> Date: Mon, 28 Jun 2021 15:12:03 +0800 Subject: [PATCH] Machanism for filter annotation&label passed from Application to workload and trait (#1843) * Add two filter * only filter in app->ac and ac->other * move filter logic to assemble phase * simplify, add indication * simplify trim * add unit test * clean up testdata --- .../application/application_controller.go | 5 +- .../v1alpha2/application/assemble/assemble.go | 49 +++++++-- .../assemble/assemble_suite_test.go | 55 ++++++++++ .../assemble/testdata/filter_annotations.yaml | 103 ++++++++++++++++++ pkg/oam/labels.go | 6 + 5 files changed, 202 insertions(+), 16 deletions(-) create mode 100644 pkg/controller/core.oam.dev/v1alpha2/application/assemble/testdata/filter_annotations.yaml diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go index 29d1f8cfd..52d7888ea 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go @@ -97,10 +97,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { Name: req.Name, Namespace: req.Namespace, }, app); err != nil { - if kerrors.IsNotFound(err) { - err = nil - } - return ctrl.Result{}, err + return ctrl.Result{}, client.IgnoreNotFound(err) } ctx = oamutil.SetNamespaceInCtx(ctx, app.Namespace) if len(app.GetAnnotations()[oam.AnnotationKubeVelaVersion]) == 0 { diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble.go b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble.go index a959ea059..a2334c706 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble.go @@ -17,6 +17,8 @@ limitations under the License. package assemble import ( + "strings" + runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/pkg/errors" @@ -31,6 +33,15 @@ import ( "github.com/oam-dev/kubevela/pkg/oam/util" ) +// DefaultFilterAnnots are annotations won't pass to workload or trait +var DefaultFilterAnnots = []string{ + oam.AnnotationAppRollout, + oam.AnnotationRollingComponent, + oam.AnnotationInplaceUpgrade, + oam.AnnotationFilterLabelKeys, + oam.AnnotationFilterAnnotationKeys, +} + // NewAppManifests create a AppManifests func NewAppManifests(appRevision *v1beta1.ApplicationRevision) *AppManifests { return &AppManifests{AppRevision: appRevision} @@ -163,7 +174,7 @@ func (am *AppManifests) assemble() { for _, comp := range am.componentManifests { compRevisionName := comp.RevisionName compName := comp.Name - commonLabels := am.generateCommonLabels(compName, compRevisionName) + commonLabels := am.generateAndFilterCommonLabels(compName, compRevisionName) klog.InfoS("Assemble manifests for component", "name", compName) wl, err := am.assembleWorkload(compName, comp.StandardWorkload, commonLabels, comp.PackagedWorkloadResources) if err != nil { @@ -236,7 +247,12 @@ func (am *AppManifests) validate() error { } // workload and trait in the same component both have these labels -func (am *AppManifests) generateCommonLabels(compName, compRevisionName string) map[string]string { +func (am *AppManifests) generateAndFilterCommonLabels(compName, compRevisionName string) map[string]string { + filter := func(labels map[string]string, notAllowedKey []string) { + for _, l := range notAllowedKey { + delete(labels, strings.TrimSpace(l)) + } + } Labels := map[string]string{ oam.LabelAppName: am.appName, oam.LabelAppRevision: am.AppRevision.Name, @@ -244,20 +260,29 @@ func (am *AppManifests) generateCommonLabels(compName, compRevisionName string) oam.LabelAppComponent: compName, oam.LabelAppComponentRevision: compRevisionName, } - // pass application's all labels to workload/trait - return util.MergeMapOverrideWithDst(Labels, am.appLabels) + // merge application's all labels + finalLabels := util.MergeMapOverrideWithDst(Labels, am.appLabels) + filterLabels, ok := am.appAnnotations[oam.AnnotationFilterLabelKeys] + if ok { + filter(finalLabels, strings.Split(filterLabels, ",")) + } + return finalLabels } // workload and trait both have these annotations -func (am *AppManifests) setAnnotations(obj *unstructured.Unstructured) { +func (am *AppManifests) filterAndSetAnnotations(obj *unstructured.Unstructured) { + var allFilterAnnotation []string + allFilterAnnotation = append(allFilterAnnotation, DefaultFilterAnnots...) + + passedFilterAnnotation, ok := am.appAnnotations[oam.AnnotationFilterAnnotationKeys] + if ok { + allFilterAnnotation = append(allFilterAnnotation, strings.Split(passedFilterAnnotation, ",")...) + } + // pass application's all annotations util.AddAnnotations(obj, am.appAnnotations) // remove useless annotations for workload/trait - util.RemoveAnnotations(obj, []string{ - oam.AnnotationAppRollout, - oam.AnnotationRollingComponent, - oam.AnnotationInplaceUpgrade, - }) + util.RemoveAnnotations(obj, allFilterAnnotation) } func (am *AppManifests) setNamespace(obj *unstructured.Unstructured) { @@ -274,7 +299,7 @@ func (am *AppManifests) assembleWorkload(compName string, wl *unstructured.Unstr // override the name set in render phase if exist wl.SetName(compName) am.setWorkloadLabels(wl, labels) - am.setAnnotations(wl) + am.filterAndSetAnnotations(wl) am.setNamespace(wl) workloadType := wl.GetLabels()[oam.WorkloadTypeLabel] @@ -319,7 +344,7 @@ func (am *AppManifests) assembleTrait(trait *unstructured.Unstructured, compName trait.SetName(traitName) } am.setTraitLabels(trait, labels) - am.setAnnotations(trait) + am.filterAndSetAnnotations(trait) am.setNamespace(trait) klog.InfoS("Successfully assemble a trait", "trait", klog.KObj(trait), "APIVersion", trait.GetAPIVersion(), "Kind", trait.GetKind()) return trait diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble_suite_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble_suite_test.go index 1427d6bae..7d5f19ba7 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble_suite_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/assemble_suite_test.go @@ -151,4 +151,59 @@ var _ = Describe("Test Assemble Options", func() { } Expect(wlScope).Should(Equal(wantScopeRef)) }) + + It("test annotation and label filter", func() { + var ( + compName = "frontend" + ) + appRev := &v1beta1.ApplicationRevision{} + b, err := ioutil.ReadFile("./testdata/filter_annotations.yaml") + getKeys := func(m map[string]string) []string { + var keys []string + for k := range m { + keys = append(keys, k) + } + return keys + } + //this appRev is generated on below this app: + /* + metadata: + name: website + annotations: + filter.oam.dev/annotation-keys: "notPassAnno1, notPassAnno2" + filter.oam.dev/label-keys: "notPassLabel" + notPassAnno1: "Annotation-filtered" + notPassAnno2: "Annotation-filtered" + canPassAnno: "Annotation-passed" + labels: + notPassLabel: "Label-filtered" + canPassLabel: "Label-passed" + spec: + components: + - name: frontend + type: webservice + properties: + image: nginx + */ + + Expect(err).Should(BeNil()) + err = yaml.Unmarshal(b, appRev) + Expect(err).Should(BeNil()) + + ao := NewAppManifests(appRev) + workloads, _, _, err := ao.GroupAssembledManifests() + Expect(err).Should(BeNil()) + + By("verify labels specified should be filtered") + wl := workloads[compName] + labelKeys := getKeys(wl.GetLabels()) + + Expect(labelKeys).ShouldNot(ContainElements("notPassLabel")) + Expect(labelKeys).Should(ContainElements("canPassLabel")) + + By("verify annotations specified should be filtered") + annotationKeys := getKeys(wl.GetAnnotations()) + Expect(annotationKeys).ShouldNot(ContainElements("notPassAnno1", "notPassAnno2")) + Expect(annotationKeys).Should(ContainElements("canPassAnno")) + }) }) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/assemble/testdata/filter_annotations.yaml b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/testdata/filter_annotations.yaml new file mode 100644 index 000000000..08e503fbf --- /dev/null +++ b/pkg/controller/core.oam.dev/v1alpha2/application/assemble/testdata/filter_annotations.yaml @@ -0,0 +1,103 @@ +apiVersion: core.oam.dev/v1beta1 +kind: ApplicationRevision +metadata: + annotations: + canPassAnno: Annotation-passed + filter.oam.dev/annotation-keys: notPassAnno1, notPassAnno2 + filter.oam.dev/label-keys: notPassLabel + kubectl.kubernetes.io/last-applied-configuration: | + {"apiVersion":"core.oam.dev/v1beta1","kind":"Application","metadata":{"annotations":{"canPassAnno":"Annotation-passed","filter.oam.dev/annotation-keys":"notPassAnno1, notPassAnno2","filter.oam.dev/label-keys":"notPassLabel","notPassAnno1":"Annotation-filtered","notPassAnno2":"Annotation-filtered"},"labels":{"canPassLabel":"Label-passed","notPassLabel":"Label-filtered"},"name":"website","namespace":"default"},"spec":{"components":[{"name":"frontend","properties":{"image":"nginx"},"type":"webservice"}]}} + notPassAnno1: Annotation-filtered + notPassAnno2: Annotation-filtered + oam.dev/kubevela-version: UNKNOWN + labels: + app.oam.dev/app-revision-hash: 1a087b23fa07c15f + app.oam.dev/name: website + canPassLabel: Label-passed + notPassLabel: Label-filtered + name: website-v1 + namespace: default + ownerReferences: + - apiVersion: core.oam.dev/v1beta1 + controller: true + kind: Application + name: website + uid: 5bd96a5d-f8fb-4d61-9345-0cca63869882 +spec: + application: + apiVersion: core.oam.dev/v1beta1 + kind: Application + metadata: + creationTimestamp: null + spec: + components: + - name: frontend + properties: + image: nginx + type: webservice + status: + rollout: + batchRollingState: "" + currentBatch: 0 + lastTargetAppRevision: "" + rollingState: "" + upgradedReadyReplicas: 0 + upgradedReplicas: 0 + applicationConfiguration: + apiVersion: core.oam.dev/v1alpha2 + kind: ApplicationConfiguration + metadata: {} + spec: + components: + - componentName: frontend + revisionName: frontend-v1 + status: + dependency: {} + observedGeneration: 0 + componentDefinitions: + webservice: + apiVersion: core.oam.dev/v1beta1 + kind: ComponentDefinition + metadata: + creationTimestamp: null + spec: + schematic: + cue: + template: "output: {\n\tapiVersion: \"apps/v1\"\n\tkind: \"Deployment\"\n\tspec: {\n\t\tselector: matchLabels: {\n\t\t\t\"app.oam.dev/component\": context.name\n\t\t\tif parameter.addRevisionLabel {\n\t\t\t\t\"app.oam.dev/appRevision\": context.appRevision\n\t\t\t}\n\t\t}\n\n\t\ttemplate: {\n\t\t\tmetadata: labels: {\n\t\t\t\t\"app.oam.dev/component\": context.name\n\t\t\t\tif parameter.addRevisionLabel {\n\t\t\t\t\t\"app.oam.dev/appRevision\": context.appRevision\n\t\t\t\t}\n\t\t\t}\n\n\t\t\tspec: {\n\t\t\t\tcontainers: [{\n\t\t\t\t\tname: context.name\n\t\t\t\t\timage: parameter.image\n\t\t\t\t\tports: [{\n\t\t\t\t\t\tcontainerPort: parameter.port\n\t\t\t\t\t}]\n\n\t\t\t\t\tif parameter[\"cmd\"] != _|_ {\n\t\t\t\t\t\tcommand: parameter.cmd\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"env\"] != _|_ {\n\t\t\t\t\t\tenv: parameter.env\n\t\t\t\t\t}\n\n\t\t\t\t\tif context[\"config\"] != _|_ {\n\t\t\t\t\t\tenv: context.config\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"cpu\"] != _|_ {\n\t\t\t\t\t\tresources: {\n\t\t\t\t\t\t\tlimits:\n\t\t\t\t\t\t\t\tcpu: parameter.cpu\n\t\t\t\t\t\t\trequests:\n\t\t\t\t\t\t\t\tcpu: parameter.cpu\n\t\t\t\t\t\t}\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"memory\"] != _|_ {\n\t\t\t\t\t\tresources: {\n\t\t\t\t\t\t\tlimits:\n\t\t\t\t\t\t\t\tmemory: parameter.memory\n\t\t\t\t\t\t\trequests:\n\t\t\t\t\t\t\t\tmemory: parameter.memory\n\t\t\t\t\t\t}\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"volumes\"] != _|_ {\n\t\t\t\t\t\tvolumeMounts: [ for v in parameter.volumes {\n\t\t\t\t\t\t\t{\n\t\t\t\t\t\t\t\tmountPath: v.mountPath\n\t\t\t\t\t\t\t\tname: v.name\n\t\t\t\t\t\t\t}}]\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"livenessProbe\"] != _|_ {\n\t\t\t\t\t\tlivenessProbe: parameter.livenessProbe\n\t\t\t\t\t}\n\n\t\t\t\t\tif parameter[\"readinessProbe\"] != _|_ {\n\t\t\t\t\t\treadinessProbe: parameter.readinessProbe\n\t\t\t\t\t}\n\n\t\t\t\t}]\n\n\t\t\tif parameter[\"volumes\"] != _|_ {\n\t\t\t\tvolumes: [ for v in parameter.volumes {\n\t\t\t\t\t{\n\t\t\t\t\t\tname: v.name\n\t\t\t\t\t\tif v.type == \"pvc\" {\n\t\t\t\t\t\t\tpersistentVolumeClaim: {\n\t\t\t\t\t\t\t\tclaimName: v.claimName\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t}\n\t\t\t\t\t\tif v.type == \"configMap\" {\n\t\t\t\t\t\t\tconfigMap: {\n\t\t\t\t\t\t\t\tdefaultMode: v.defaultMode\n\t\t\t\t\t\t\t\tname: v.cmName\n\t\t\t\t\t\t\t\tif v.items != _|_ {\n\t\t\t\t\t\t\t\t\titems: v.items\n\t\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t}\n\t\t\t\t\t\tif v.type == \"secret\" {\n\t\t\t\t\t\t\tsecret: {\n\t\t\t\t\t\t\t\tdefaultMode: v.defaultMode\n\t\t\t\t\t\t\t\tsecretName: v.secretName\n\t\t\t\t\t\t\t\tif v.items != _|_ {\n\t\t\t\t\t\t\t\t\titems: v.items\n\t\t\t\t\t\t\t\t}\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t}\n\t\t\t\t\t\tif v.type == \"emptyDir\" {\n\t\t\t\t\t\t\temptyDir: {\n\t\t\t\t\t\t\t\tmedium: v.medium\n\t\t\t\t\t\t\t}\n\t\t\t\t\t\t}\n\t\t\t\t\t}}]\n\t\t\t}\n\t\t}\n\t}\n}\n}\nparameter: {\n\t// +usage=Which image would you like to use for your service\n\t// +short=i\n\timage: string\n\n\t// +usage=Which port do you want customer traffic sent to\n\t// +short=p\n\tport: *80 | int\n\n\t// +ignore\n\t// +usage=If addRevisionLabel is true, the appRevision label will be added to the underlying pods\n\taddRevisionLabel: *false | bool\n\n\t// +usage=Commands to run in the container\n\tcmd?: [...string]\n\n\t// +usage=Define arguments by using environment variables\n\tenv?: [...{\n\t\t// +usage=Environment variable name\n\t\tname: string\n\t\t// +usage=The value of the environment variable\n\t\tvalue?: string\n\t\t// +usage=Specifies a source the value of this var should come from\n\t\tvalueFrom?: {\n\t\t\t// +usage=Selects a key of a secret in the pod's namespace\n\t\t\tsecretKeyRef: {\n\t\t\t\t// +usage=The name of the secret in the pod's namespace to select from\n\t\t\t\tname: string\n\t\t\t\t// +usage=The key of the secret to select from. Must be a valid secret key\n\t\t\t\tkey: string\n\t\t\t}\n\t\t}\n\t}]\n\n\t// +usage=Number of CPU units for the service, like `0.5` (0.5 CPU core), `1` (1 CPU core)\n\tcpu?: string\n\n\t// +usage=Specifies the attributes of the memory resource required for the container.\n\tmemory?: string\n\n\t// +usage=Declare volumes and volumeMounts\n\tvolumes?: [...{\n\t\tname: string\n\t\tmountPath: string\n\t\t// +usage=Specify volume type, options: \"pvc\",\"configMap\",\"secret\",\"emptyDir\"\n\t\ttype: \"pvc\" | \"configMap\" | \"secret\" | \"emptyDir\"\n\t\tif type == \"pvc\" {\n\t\t\tclaimName: string\n\t\t}\n\t\tif type == \"configMap\" {\n\t\t\tdefaultMode: *420 | int\n\t\t\tcmName: string\n\t\t\titems?: [...{\n\t\t\t\tkey: string\n\t\t\t\tpath: string\n\t\t\t\tmode: *511 | int\n\t\t\t}]\n\t\t}\n\t\tif type == \"secret\" {\n\t\t\tdefaultMode: *420 | int\n\t\t\tsecretName: string\n\t\t\titems?: [...{\n\t\t\t\tkey: string\n\t\t\t\tpath: string\n\t\t\t\tmode: *511 | int\n\t\t\t}]\n\t\t}\n\t\tif type == \"emptyDir\" {\n\t\t\tmedium: *\"\" | \"Memory\"\n\t\t}\n\t}]\n\n\t// +usage=Instructions for assessing whether the container is alive.\n\tlivenessProbe?: #HealthProbe\n\n\t// +usage=Instructions for assessing whether the container is in a suitable state to serve traffic.\n\treadinessProbe?: #HealthProbe\n}\n\n#HealthProbe: {\n\n\t// +usage=Instructions for assessing container health by executing a command. Either this attribute or the httpGet attribute or the tcpSocket attribute MUST be specified. This attribute is mutually exclusive with both the httpGet attribute and the tcpSocket attribute.\n\texec?: {\n\t\t// +usage=A command to be executed inside the container to assess its health. Each space delimited token of the command is a separate array element. Commands exiting 0 are considered to be successful probes, whilst all other exit codes are considered failures.\n\t\tcommand: [...string]\n\t}\n\n\t// +usage=Instructions for assessing container health by executing an HTTP GET request. Either this attribute or the exec attribute or the tcpSocket attribute MUST be specified. This attribute is mutually exclusive with both the exec attribute and the tcpSocket attribute.\n\thttpGet?: {\n\t\t// +usage=The endpoint, relative to the port, to which the HTTP GET request should be directed.\n\t\tpath: string\n\t\t// +usage=The TCP socket within the container to which the HTTP GET request should be directed.\n\t\tport: int\n\t\thttpHeaders?: [...{\n\t\t\tname: string\n\t\t\tvalue: string\n\t\t}]\n\t}\n\n\t// +usage=Instructions for assessing container health by probing a TCP socket. Either this attribute or the exec attribute or the httpGet attribute MUST be specified. This attribute is mutually exclusive with both the exec attribute and the httpGet attribute.\n\ttcpSocket?: {\n\t\t// +usage=The TCP socket within the container that should be probed to assess container health.\n\t\tport: int\n\t}\n\n\t// +usage=Number of seconds after the container is started before the first probe is initiated.\n\tinitialDelaySeconds: *0 | int\n\n\t// +usage=How often, in seconds, to execute the probe.\n\tperiodSeconds: *10 | int\n\n\t// +usage=Number of seconds after which the probe times out.\n\ttimeoutSeconds: *1 | int\n\n\t// +usage=Minimum consecutive successes for the probe to be considered successful after having failed.\n\tsuccessThreshold: *1 | int\n\n\t// +usage=Number of consecutive failures required to determine the container is not alive (liveness probe) or not ready (readiness probe).\n\tfailureThreshold: *3 | int\n}\n" + workload: + definition: + apiVersion: apps/v1 + kind: Deployment + status: {} + components: + - raw: + apiVersion: core.oam.dev/v1alpha2 + kind: Component + metadata: + labels: + app.oam.dev/component-revision-hash: 943da166b084e088 + name: frontend + spec: + workload: + apiVersion: apps/v1 + kind: Deployment + metadata: + labels: + app.oam.dev/appRevision: website-v1 + app.oam.dev/component: frontend + app.oam.dev/name: website + workload.oam.dev/type: webservice + spec: + selector: + matchLabels: + app.oam.dev/component: frontend + template: + metadata: + labels: + app.oam.dev/component: frontend + spec: + containers: + - image: nginx + name: frontend + ports: + - containerPort: 80 diff --git a/pkg/oam/labels.go b/pkg/oam/labels.go index 36f264426..59697d593 100644 --- a/pkg/oam/labels.go +++ b/pkg/oam/labels.go @@ -99,4 +99,10 @@ const ( // AnnotationKubeVelaVersion is used to record current KubeVela version AnnotationKubeVelaVersion = "oam.dev/kubevela-version" + + // AnnotationFilterAnnotationKeys is used to filter annotations passed to workload and trait, split by comma + AnnotationFilterAnnotationKeys = "filter.oam.dev/annotation-keys" + + // AnnotationFilterLabelKeys is used to filter labels passed to workload and trait, split by comma + AnnotationFilterLabelKeys = "filter.oam.dev/label-keys" )