Skip to content

Commit

Permalink
wip: introduce v1alpha2 api with auto conversion
Browse files Browse the repository at this point in the history
This introduces a new api version `v1alpha2` while still allowing user
to use `v1alpha1` API.

- Only one version of the CRD can be stored, so `v1alpha1` is the one
stored
- All the version of the CRD **must** be schema compatible (before 1.16)
  - `v1alpha1` gets **all** the fields, meaning :
    - the new ones (from `v1alpha2`, …)
    - the old deprecated ones (that will be removed from `v1alpha2`)
  - we may *only* document the "frozen" `v1alpha1` field (and not the
    new one that are coming from the other api versions)
- `kubectl` get returns the highest priority version, aka the most
  recent one, here `v1alpha2`. To make *sure* any object created from
  any version are valid (while getting), we convert them using the
  admission webhook (#blackmagic 🧙).
  - Object created using `v1alpha1` API will automatically be
  converted (see below)
  - This has the downsize of not being able to `get` v1alpha1 object
  anymore (although it should be possible using the `client-go` and
  some even more #blackmagic 🧙 hack on the webhook side probably)
- One benefit of this approach (`apis.Convertible`) is that once
  we *only* support 1.16 (the release with conversion webhook), this
  will be usable almost as is (for the conversion webhook) 👼.

This is done in a quick and dirty way:
- Lot's of code is duplicated where it would not be needed, it was
  just quicker to do so at that point in time
- I completely skipped the *variable interpolation* validation for
  `Parameters` as those values would/will need to be automatically
  transformed too (but it was a bit complicated for what I wanted to
  experiment on)
- I did not implement the **reconcilier** part of this. It should be
  simple as:
  - it will use only one object (the store one, aka `v1alpha1`)
  - it will make the assumption that the objects are fully
    transformed (either by the webhook or in-memory by the
    controller), meaning there will be no *handling
    backward-compatible* piece of code in the reconcilier (all is
    contained in the `Validable`, `Defaultable` and `Convertible` impl.).

Possible next steps are:
- Split this into smaller PR.
- Introduce a `v1alpha2` package early but not exposing it before we
  are ready to. This will allow us to not have to rush this for
  0.9, **and** to increment over this in smaller step.
- Decide what `v1alpha2` should look like — I did this experiment with
  `TaskSpec.Inputs.Params` to `TaskSpec.Params` but we should decide
  what we want to change and target that.
- Reduce duplication by reusing anything that didn't change from
  `v1alpha1` to `v1alpha2` in `v1alpha1` (aka refering the same type
  in go)
- Related to this experiment, implement the variable interpolation
  validation thing **and** the reconcilier part too.
- Need to figure out if the definition types (`Task`, `Pipeline`, …)
  need a `controller` (I don't think so but…)

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
  • Loading branch information
vdemeester committed Nov 6, 2019
1 parent f15a0f3 commit d75b034
Show file tree
Hide file tree
Showing 160 changed files with 17,284 additions and 9,631 deletions.
11 changes: 10 additions & 1 deletion cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

apiconfig "github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha2"
tklogging "github.com/tektoncd/pipeline/pkg/logging"
"github.com/tektoncd/pipeline/pkg/system"
"go.uber.org/zap"
Expand Down Expand Up @@ -95,6 +96,14 @@ func main() {
v1alpha1.SchemeGroupVersion.WithKind("TaskRun"): &v1alpha1.TaskRun{},
v1alpha1.SchemeGroupVersion.WithKind("PipelineRun"): &v1alpha1.PipelineRun{},
v1alpha1.SchemeGroupVersion.WithKind("Condition"): &v1alpha1.Condition{},

v1alpha2.SchemeGroupVersion.WithKind("Pipeline"): &v1alpha2.Pipeline{},
v1alpha2.SchemeGroupVersion.WithKind("PipelineResource"): &v1alpha2.PipelineResource{},
v1alpha2.SchemeGroupVersion.WithKind("Task"): &v1alpha2.Task{},
v1alpha2.SchemeGroupVersion.WithKind("ClusterTask"): &v1alpha2.ClusterTask{},
v1alpha2.SchemeGroupVersion.WithKind("TaskRun"): &v1alpha2.TaskRun{},
v1alpha2.SchemeGroupVersion.WithKind("PipelineRun"): &v1alpha2.PipelineRun{},
v1alpha2.SchemeGroupVersion.WithKind("Condition"): &v1alpha2.Condition{},
}

resourceAdmissionController := webhook.NewResourceAdmissionController(resourceHandlers, options, true)
Expand All @@ -104,7 +113,7 @@ func main() {

// Decorate contexts with the current state of the config.
ctxFunc := func(ctx context.Context) context.Context {
return v1alpha1.WithDefaultConfigurationName(store.ToContext(ctx))
return v1alpha2.WithUpgradeViaDefaulting(store.ToContext(ctx))
}

controller, err := webhook.New(kubeClient, options, admissionControllers, logger, ctxFunc)
Expand Down
8 changes: 7 additions & 1 deletion config/300-clustertask.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: clustertasks.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: ClusterTask
plural: clustertasks
Expand All @@ -28,4 +35,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
8 changes: 7 additions & 1 deletion config/300-condition.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: conditions.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: Condition
plural: conditions
Expand All @@ -28,4 +35,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
8 changes: 7 additions & 1 deletion config/300-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: pipelines.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: Pipeline
plural: pipelines
Expand All @@ -28,4 +35,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
8 changes: 7 additions & 1 deletion config/300-pipelinerun.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: pipelineruns.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: PipelineRun
plural: pipelineruns
Expand Down Expand Up @@ -44,4 +51,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
8 changes: 7 additions & 1 deletion config/300-resource.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: pipelineresources.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: PipelineResource
plural: pipelineresources
Expand All @@ -28,4 +35,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
8 changes: 7 additions & 1 deletion config/300-task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: tasks.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: Task
plural: tasks
Expand All @@ -28,4 +35,3 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
9 changes: 8 additions & 1 deletion config/300-taskrun.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ metadata:
name: taskruns.tekton.dev
spec:
group: tekton.dev
versions:
- name: v1alpha1
served: true
storage: true
- name: v1alpha2
served: true
storage: false
names:
kind: TaskRun
plural: taskruns
Expand Down Expand Up @@ -44,4 +51,4 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1

34 changes: 17 additions & 17 deletions examples/pipelineruns/pipelinerun.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: PipelineResource
metadata:
name: skaffold-image-leeroy-app
Expand All @@ -26,7 +26,7 @@ roleRef:
name: cluster-admin
apiGroup: rbac.authorization.k8s.io
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: PipelineResource
metadata:
name: skaffold-image-leeroy-web-pipelinerun
Expand All @@ -36,7 +36,7 @@ spec:
- name: url
value: gcr.io/christiewilson-catfactory/leeroy-web
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: PipelineResource
metadata:
name: skaffold-git-pipelinerun
Expand All @@ -48,7 +48,7 @@ spec:
- name: url
value: https://github.com/GoogleContainerTools/skaffold
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: Task
metadata:
name: unit-tests
Expand All @@ -70,7 +70,7 @@ spec:
args:
- "pass"
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: Task
metadata:
name: build-push
Expand All @@ -79,13 +79,13 @@ spec:
resources:
- name: workspace
type: git
params:
- name: pathToDockerFile
description: The path to the dockerfile to build
default: /workspace/workspace/Dockerfile
- name: pathToContext
description: The build context used by Kaniko (https://github.com/GoogleContainerTools/kaniko#kaniko-build-contexts)
default: /workspace/workspace
params:
- name: pathToDockerFile
description: The path to the dockerfile to build
default: /workspace/workspace/Dockerfile
- name: pathToContext
description: The build context used by Kaniko (https://github.com/GoogleContainerTools/kaniko#kaniko-build-contexts)
default: /workspace/workspace
outputs:
resources:
- name: builtImage
Expand All @@ -100,9 +100,9 @@ spec:
command:
- /kaniko/executor
args:
- --dockerfile=$(inputs.params.pathToDockerFile)
- --dockerfile=$(params.pathToDockerFile)
- --destination=$(outputs.resources.builtImage.url)
- --context=$(inputs.params.pathToContext)
- --context=$(params.pathToContext)
---
#This task deploys with kubectl apply -f <filename>
apiVersion: tekton.dev/v1alpha1
Expand Down Expand Up @@ -131,7 +131,7 @@ spec:
- "w"
- "-i"
- "$(inputs.params.yqArg)"
- "$(inputs.params.path)"
- "$kube(inputs.params.path)"
- "$(inputs.params.yamlPathToImage)"
- "$(inputs.resources.image.url)"
- name: run-kubectl
Expand All @@ -150,7 +150,7 @@ spec:
# pushed are the ones that are deployed (that would require using the digest of
# the built image, see https://github.com/tektoncd/pipeline/issues/216).

apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: Pipeline
metadata:
name: demo-pipeline
Expand Down Expand Up @@ -239,7 +239,7 @@ spec:
- name: yamlPathToImage
value: "spec.template.spec.containers[0].image"
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: PipelineRun
metadata:
name: demo-pipeline-run-1
Expand Down
51 changes: 29 additions & 22 deletions examples/taskruns/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,36 @@ metadata:
data:
test.data: tasks are my jam
---
apiVersion: tekton.dev/v1alpha1
apiVersion: tekton.dev/v1alpha2
kind: Task
metadata:
name: foo
spec:
steps:
- name: secret
image: ubuntu
command: ['bash']
args:
- '-c'
- '[[ $(cat /config/test.data) == $TEST_DATA ]]'
env:
- name: TEST_DATA
valueFrom:
configMapKeyRef:
name: config-for-testing-configmaps
key: test.data
volumeMounts:
- name: config-volume
mountPath: /config
volumes:
- name: config-volume
configMap:
name: config-for-testing-configmaps
---
apiVersion: tekton.dev/v1alpha2
kind: TaskRun
metadata:
generateName: configmap-
spec:
taskSpec:
steps:
- name: secret
image: ubuntu
command: ['bash']
args:
- '-c'
- '[[ $(cat /config/test.data) == $TEST_DATA ]]'
env:
- name: TEST_DATA
valueFrom:
configMapKeyRef:
name: config-for-testing-configmaps
key: test.data
volumeMounts:
- name: config-volume
mountPath: /config
volumes:
- name: config-volume
configMap:
name: config-for-testing-configmaps
taskRef:
name: foo
4 changes: 2 additions & 2 deletions hack/update-codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ KNATIVE_CODEGEN_PKG=${KNATIVE_CODEGEN_PKG:-$(cd ${REPO_ROOT_DIR}; ls -d -1 ./ven
# instead of the $GOPATH directly. For normal projects this can be dropped.
${CODEGEN_PKG}/generate-groups.sh "deepcopy,client,informer,lister" \
github.com/tektoncd/pipeline/pkg/client github.com/tektoncd/pipeline/pkg/apis \
pipeline:v1alpha1 \
pipeline:v1alpha1,v1alpha2 \
--go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt

# Depends on generate-groups.sh to install bin/deepcopy-gen
Expand All @@ -42,7 +42,7 @@ ${GOPATH}/bin/deepcopy-gen \
# Knative Injection
${KNATIVE_CODEGEN_PKG}/hack/generate-knative.sh "injection" \
github.com/tektoncd/pipeline/pkg/client github.com/tektoncd/pipeline/pkg/apis \
"pipeline:v1alpha1" \
"pipeline:v1alpha1,v1alpha2" \
--go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt

# Make sure our dependencies are up-to-date
Expand Down
Loading

0 comments on commit d75b034

Please sign in to comment.