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.

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

Tekton release manifest is not working with kustomize and ytt #3794

Closed
mhubig opened this issue Feb 26, 2021 · 2 comments · Fixed by #3842
Closed

Tekton release manifest is not working with kustomize and ytt #3794

mhubig opened this issue Feb 26, 2021 · 2 comments · Fixed by #3842
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@mhubig
Copy link

mhubig commented Feb 26, 2021

Expected Behavior

The tekton release manifest should work out of the box with tools like kustomize and ytt.

Actual Behavior

Both ytt and kustomize crashes with the current tekton release manifest. See kubernetes-sigs/kustomize#3639 and carvel-dev/ytt#310 for reference.

Steps to Reproduce the Problem

I think this is because of the strange merge syntax within the manifest:

- !!merge <<: *version
   name: v1beta1
   storage: true

Maybe this can be replaced in future releases with a rendered version without the merge syntax. Here is a diff with the rendered version of the merge syntax:

diff --git a/base/tekton.yaml b/base/tekton.yaml
index 13841da..f2ae1dc 100644
--- a/base/tekton.yaml
+++ b/base/tekton.yaml
@@ -477,8 +477,7 @@ spec:
   group: tekton.dev
   preserveUnknownFields: false
   versions:
-    - &version
-      name: v1alpha1
+    - name: v1alpha1
       served: true
       storage: false
       schema:
@@ -496,9 +495,24 @@ spec:
       # starts to increment
       subresources:
         status: {}
-    - !!merge <<: *version
-      name: v1beta1
+    - name: v1beta1
+      served: true
       storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          # One can use x-kubernetes-preserve-unknown-fields: true
+          # at the root of the schema (and inside any properties, additionalProperties)
+          # to get the traditional CRD behaviour that nothing is pruned, despite
+          # setting spec.preserveUnknownProperties: false.
+          #
+          # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
+          # See issue: https://github.com/knative/serving/issues/912
+          x-kubernetes-preserve-unknown-fields: true
+      # Opt into the status subresource so metadata.generation
+      # starts to increment
+      subresources:
+        status: {}
   names:
     kind: ClusterTask
     plural: clustertasks
@@ -635,8 +649,7 @@ spec:
   group: tekton.dev
   preserveUnknownFields: false
   versions:
-    - &version
-      name: v1alpha1
+    - name: v1alpha1
       served: true
       storage: false
       # Opt into the status subresource so metadata.generation
@@ -654,9 +667,24 @@ spec:
           # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
           # See issue: https://github.com/knative/serving/issues/912
           x-kubernetes-preserve-unknown-fields: true
-    - !!merge <<: *version
-      name: v1beta1
+    - name: v1beta1
+      served: true
       storage: true
+      # Opt into the status subresource so metadata.generation
+      # starts to increment
+      subresources:
+        status: {}
+      schema:
+        openAPIV3Schema:
+          type: object
+          # One can use x-kubernetes-preserve-unknown-fields: true
+          # at the root of the schema (and inside any properties, additionalProperties)
+          # to get the traditional CRD behaviour that nothing is pruned, despite
+          # setting spec.preserveUnknownProperties: false.
+          #
+          # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
+          # See issue: https://github.com/knative/serving/issues/912
+          x-kubernetes-preserve-unknown-fields: true
   names:
     kind: Pipeline
     plural: pipelines
@@ -701,8 +729,7 @@ spec:
   group: tekton.dev
   preserveUnknownFields: false
   versions:
-    - &version
-      name: v1alpha1
+    - name: v1alpha1
       served: true
       storage: false
       schema:
@@ -733,9 +760,37 @@ spec:
       # starts to increment
       subresources:
         status: {}
-    - !!merge <<: *version
-      name: v1beta1
+    - name: v1beta1
+      served: true
       storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          # One can use x-kubernetes-preserve-unknown-fields: true
+          # at the root of the schema (and inside any properties, additionalProperties)
+          # to get the traditional CRD behaviour that nothing is pruned, despite
+          # setting spec.preserveUnknownProperties: false.
+          #
+          # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
+          # See issue: https://github.com/knative/serving/issues/912
+          x-kubernetes-preserve-unknown-fields: true
+      additionalPrinterColumns:
+        - name: Succeeded
+          type: string
+          jsonPath: ".status.conditions[?(@.type==\"Succeeded\")].status"
+        - name: Reason
+          type: string
+          jsonPath: ".status.conditions[?(@.type==\"Succeeded\")].reason"
+        - name: StartTime
+          type: date
+          jsonPath: .status.startTime
+        - name: CompletionTime
+          type: date
+          jsonPath: .status.completionTime
+      # Opt into the status subresource so metadata.generation
+      # starts to increment
+      subresources:
+        status: {}
   names:
     kind: PipelineRun
     plural: pipelineruns
@@ -903,8 +958,7 @@ spec:
   group: tekton.dev
   preserveUnknownFields: false
   versions:
-    - &version
-      name: v1alpha1
+    - name: v1alpha1
       served: true
       storage: false
       schema:
@@ -922,9 +976,24 @@ spec:
       # starts to increment
       subresources:
         status: {}
-    - !!merge <<: *version
-      name: v1beta1
+    - name: v1beta1
+      served: true
       storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          # One can use x-kubernetes-preserve-unknown-fields: true
+          # at the root of the schema (and inside any properties, additionalProperties)
+          # to get the traditional CRD behaviour that nothing is pruned, despite
+          # setting spec.preserveUnknownProperties: false.
+          #
+          # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
+          # See issue: https://github.com/knative/serving/issues/912
+          x-kubernetes-preserve-unknown-fields: true
+      # Opt into the status subresource so metadata.generation
+      # starts to increment
+      subresources:
+        status: {}
   names:
     kind: Task
     plural: tasks
@@ -969,8 +1038,7 @@ spec:
   group: tekton.dev
   preserveUnknownFields: false
   versions:
-    - &version
-      name: v1alpha1
+    - name: v1alpha1
       served: true
       storage: false
       schema:
@@ -1001,9 +1069,37 @@ spec:
       # starts to increment
       subresources:
         status: {}
-    - !!merge <<: *version
-      name: v1beta1
+    - name: v1beta1
+      served: true
       storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          # One can use x-kubernetes-preserve-unknown-fields: true
+          # at the root of the schema (and inside any properties, additionalProperties)
+          # to get the traditional CRD behaviour that nothing is pruned, despite
+          # setting spec.preserveUnknownProperties: false.
+          #
+          # See https://kubernetes.io/blog/2019/06/20/crd-structural-schema/
+          # See issue: https://github.com/knative/serving/issues/912
+          x-kubernetes-preserve-unknown-fields: true
+      additionalPrinterColumns:
+        - name: Succeeded
+          type: string
+          jsonPath: ".status.conditions[?(@.type==\"Succeeded\")].status"
+        - name: Reason
+          type: string
+          jsonPath: ".status.conditions[?(@.type==\"Succeeded\")].reason"
+        - name: StartTime
+          type: date
+          jsonPath: .status.startTime
+        - name: CompletionTime
+          type: date
+          jsonPath: .status.completionTime
+      # Opt into the status subresource so metadata.generation
+      # starts to increment
+      subresources:
+        status: {}
   names:
     kind: TaskRun
     plural: taskruns
@mhubig mhubig added the kind/bug Categorizes issue or PR as related to a bug. label Feb 26, 2021
@ghglza
Copy link

ghglza commented Mar 17, 2021

Have you seen kustomize issue kubernetes-sigs/kustomize#3614 ?
I think at least for kustomize the crash is caused by a bug in kyaml and affect only those versions of kustomize that rely on kyaml.
kustomization of the Tekton resources (including YAML anchors and aliases!) works fine with kustomize 3.8.10

@zhouhaibing089
Copy link
Contributor

zhouhaibing089 commented Mar 26, 2021

For anyone who have to deal with it:

$ yq eval 'explode(.)' release.yaml

will expand those merge keys. yq is from here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants