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

Add support for conversion webhook 🍸 #2413

Merged
merged 2 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 69 additions & 1 deletion cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"os"

defaultconfig "github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/pkg/contexts"
Expand All @@ -37,6 +38,7 @@ import (
"knative.dev/pkg/webhook/certificates"
"knative.dev/pkg/webhook/configmaps"
"knative.dev/pkg/webhook/resourcesemantics"
"knative.dev/pkg/webhook/resourcesemantics/conversion"
"knative.dev/pkg/webhook/resourcesemantics/defaulting"
"knative.dev/pkg/webhook/resourcesemantics/validation"
)
Expand Down Expand Up @@ -85,6 +87,9 @@ func NewDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher
}

func NewValidationAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
// Decorate contexts with the current state of the config.
store := defaultconfig.NewStore(logging.FromContext(ctx).Named("config-store"))
store.WatchConfigs(cmw)
return validation.NewAdmissionController(ctx,

// Name of the resource webhook.
Expand All @@ -98,7 +103,7 @@ func NewValidationAdmissionController(ctx context.Context, cmw configmap.Watcher

// A function that infuses the context passed to Validate/SetDefaults with custom metadata.
func(ctx context.Context) context.Context {
return ctx
return contexts.WithUpgradeViaDefaulting(store.ToContext(ctx))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contexts.WithUpgradeViaDefaulting

Are you using this upgrade marker for anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we do

},

// Whether to disallow unknown fields.
Expand All @@ -124,6 +129,68 @@ func NewConfigValidationController(ctx context.Context, cmw configmap.Watcher) *
)
}

func NewConversionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
// nolint: golint
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we not want to lint this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 well, I need to find a better name than v1beta1_ for the variable.. and I was lazy 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what variable name to use to be honest 🙃

var (
v1alpha1_ = v1alpha1.SchemeGroupVersion.Version
v1beta1_ = v1beta1.SchemeGroupVersion.Version
)

return conversion.NewConversionController(ctx,
// The path on which to serve the webhook
"/resource-conversion",

// Specify the types of custom resource definitions that should be converted
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the keys are the types to convert TO and the zygotes are the version to convert FROM is that right? im not 100% clear either way tho so maybe some more detail in the comment might help?

map[schema.GroupKind]conversion.GroupKindConversion{
v1beta1.Kind("Task"): {
DefinitionName: pipeline.TaskResource.String(),
HubVersion: v1alpha1_,
Zygotes: map[string]conversion.ConvertibleObject{
v1alpha1_: &v1alpha1.Task{},
v1beta1_: &v1beta1.Task{},
},
},
v1beta1.Kind("ClusterTask"): {
DefinitionName: pipeline.ClusterTaskResource.String(),
HubVersion: v1alpha1_,
Zygotes: map[string]conversion.ConvertibleObject{
v1alpha1_: &v1alpha1.ClusterTask{},
v1beta1_: &v1beta1.ClusterTask{},
},
},
v1beta1.Kind("TaskRun"): {
DefinitionName: pipeline.TaskRunResource.String(),
HubVersion: v1alpha1_,
Zygotes: map[string]conversion.ConvertibleObject{
v1alpha1_: &v1alpha1.TaskRun{},
v1beta1_: &v1beta1.TaskRun{},
},
},
v1beta1.Kind("Pipeline"): {
DefinitionName: pipeline.PipelineResource.String(),
HubVersion: v1alpha1_,
Zygotes: map[string]conversion.ConvertibleObject{
v1alpha1_: &v1alpha1.Pipeline{},
v1beta1_: &v1beta1.Pipeline{},
},
},
v1beta1.Kind("PipelineRun"): {
DefinitionName: pipeline.PipelineRunResource.String(),
HubVersion: v1alpha1_,
Zygotes: map[string]conversion.ConvertibleObject{
v1alpha1_: &v1alpha1.PipelineRun{},
v1beta1_: &v1beta1.PipelineRun{},
},
},
},

// A function that infuses the context passed to ConvertTo/ConvertFrom/SetDefaults with custom metadata
func(ctx context.Context) context.Context {
return ctx
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want the ctx stuff here too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed what was in knative/serving 😝

},
)
}

func main() {
serviceName := os.Getenv("WEBHOOK_SERVICE_NAME")
if serviceName == "" {
Expand All @@ -146,5 +213,6 @@ func main() {
NewDefaultingAdmissionController,
NewValidationAdmissionController,
NewConfigValidationController,
NewConversionController,
)
}
5 changes: 5 additions & 0 deletions config/200-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: tekton-pipelines-webhook-cluster-access
rules:
# The webhook needs to be able to list and update customresourcedefinitions,
# mainly to update the webhook certificates.
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions", "customresourcedefinitions/status"]
verbs: ["get", "list", "update", "patch", "watch"]
- apiGroups: ["admissionregistration.k8s.io"]
# The webhook performs a reconciliation on these two resources and continuously
# updates configuration.
Expand Down
19 changes: 18 additions & 1 deletion config/300-clustertask.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ metadata:
version: "devel"
spec:
group: tekton.dev
preserveUnknownFields: false
validation:
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
versions:
- name: v1alpha1
served: true
Expand All @@ -39,4 +51,9 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
conversion:
strategy: Webhook
webhookClientConfig:
service:
name: tekton-pipelines-webhook
namespace: tekton-pipelines
19 changes: 18 additions & 1 deletion config/300-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ metadata:
version: "devel"
spec:
group: tekton.dev
preserveUnknownFields: false
validation:
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
versions:
- name: v1alpha1
served: true
Expand All @@ -39,4 +51,9 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
conversion:
strategy: Webhook
webhookClientConfig:
service:
name: tekton-pipelines-webhook
namespace: tekton-pipelines
19 changes: 18 additions & 1 deletion config/300-pipelinerun.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ metadata:
version: "devel"
spec:
group: tekton.dev
preserveUnknownFields: false
validation:
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
versions:
- name: v1alpha1
served: true
Expand Down Expand Up @@ -55,4 +67,9 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
conversion:
strategy: Webhook
webhookClientConfig:
service:
name: tekton-pipelines-webhook
namespace: tekton-pipelines
19 changes: 18 additions & 1 deletion config/300-task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ metadata:
version: "devel"
spec:
group: tekton.dev
preserveUnknownFields: false
validation:
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
versions:
- name: v1alpha1
served: true
Expand All @@ -39,4 +51,9 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
conversion:
strategy: Webhook
webhookClientConfig:
service:
name: tekton-pipelines-webhook
namespace: tekton-pipelines
19 changes: 18 additions & 1 deletion config/300-taskrun.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ metadata:
version: "devel"
spec:
group: tekton.dev
preserveUnknownFields: false
validation:
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
versions:
- name: v1alpha1
served: true
Expand Down Expand Up @@ -55,4 +67,9 @@ spec:
# starts to increment
subresources:
status: {}
version: v1alpha1
conversion:
strategy: Webhook
webhookClientConfig:
service:
name: tekton-pipelines-webhook
namespace: tekton-pipelines
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ require (
k8s.io/gengo v0.0.0-20191108084044-e500ee069b5c // indirect
k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a
knative.dev/caching v0.0.0-20200116200605-67bca2c83dfa
knative.dev/pkg v0.0.0-20200227193851-2fe8db300072
knative.dev/pkg v0.0.0-20200306230727-a56a6ea3fa56
)

// Knative deps (release-0.13)
replace (
contrib.go.opencensus.io/exporter/stackdriver => contrib.go.opencensus.io/exporter/stackdriver v0.12.9-0.20191108183826-59d068f8d8ff
knative.dev/caching => knative.dev/caching v0.0.0-20200116200605-67bca2c83dfa
knative.dev/pkg => knative.dev/pkg v0.0.0-20200227193851-2fe8db300072
knative.dev/pkg => knative.dev/pkg v0.0.0-20200306230727-a56a6ea3fa56
knative.dev/pkg/vendor/github.com/spf13/pflag => github.com/spf13/pflag v1.0.5
)

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -911,8 +911,8 @@ k8s.io/utils v0.0.0-20191114184206-e782cd3c129f h1:GiPwtSzdP43eI1hpPCbROQCCIgCui
k8s.io/utils v0.0.0-20191114184206-e782cd3c129f/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
knative.dev/caching v0.0.0-20200116200605-67bca2c83dfa h1:mxrur6DsVK8uIjhIq7c1OMls4YjBcRlyvnh3Vx13a0M=
knative.dev/caching v0.0.0-20200116200605-67bca2c83dfa/go.mod h1:dHXFU6CGlLlbzaWc32g80cR92iuBSpsslDNBWI8C7eg=
knative.dev/pkg v0.0.0-20200227193851-2fe8db300072 h1:ZituYY5obt+2k4JRN5hNmtX00KXDBXGMohS6n2xf250=
knative.dev/pkg v0.0.0-20200227193851-2fe8db300072/go.mod h1:pgODObA1dTyhNoFxPZTTjNWfx6F0aKsKzn+vaT9XO/Q=
knative.dev/pkg v0.0.0-20200306230727-a56a6ea3fa56 h1:o9CEa3qt7Bw6jHyB7NUVvSYZNuY4Xa3DuAqe/faBrZI=
knative.dev/pkg v0.0.0-20200306230727-a56a6ea3fa56/go.mod h1:pgODObA1dTyhNoFxPZTTjNWfx6F0aKsKzn+vaT9XO/Q=
modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
Expand Down
41 changes: 41 additions & 0 deletions pkg/apis/pipeline/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ limitations under the License.

package pipeline

import "k8s.io/apimachinery/pkg/runtime/schema"

const (
// GroupName is the Kubernetes resource group name for Pipeline types.
GroupName = "tekton.dev"
Expand All @@ -41,3 +43,42 @@ const (
// ConditionCheckKey is used as the label identifier for a ConditionCheck
ConditionCheckKey = "/conditionCheck"
)

var (
// TaskResource represents a Tekton Task
TaskResource = schema.GroupResource{
Group: GroupName,
Resource: "tasks",
}
// ClusterTaskResource represents a Tekton ClusterTask
ClusterTaskResource = schema.GroupResource{
Group: GroupName,
Resource: "clustertasks",
}
// TaskRunResource represents a Tekton TaskRun
TaskRunResource = schema.GroupResource{
Group: GroupName,
Resource: "taskruns",
}
// PipelineResource represents a Tekton Pipeline
PipelineResource = schema.GroupResource{
Group: GroupName,
Resource: "pipelines",
}
// PipelineRunResource represents a Tekton PipelineRun
PipelineRunResource = schema.GroupResource{
Group: GroupName,
Resource: "pipelineruns",
}

// PipelineResourceResource represents a Tekton PipelineResource
PipelineResourceResource = schema.GroupResource{
Group: GroupName,
Resource: "pipelineresources",
}
// ConditionResource represents a Tekton Condition
ConditionResource = schema.GroupResource{
Group: GroupName,
Resource: "conditions",
}
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not 100% clear on why these are required now or where they are used? does the conversion webhook automatically detect these?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do here : https://github.com/tektoncd/pipeline/pull/2413/files#diff-58de452513b7e8d8d3cfea23eb4ae6a8R170 (and we might do at other parts of the code). It makes it a bit easier to get conditions.tekton.dev or pipelineruns.tekton.dev in code without hard coding those.

Loading