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.

Sign up for GitHub

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

Jump to bottom

Disable implicit param behavior for Pipeline objects. #4511

Merged
merged 1 commit into from
Jan 26, 2022
Merged

Disable implicit param behavior for Pipeline objects. #4511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
473 changes: 473 additions & 0 deletions pkg/apis/pipeline/v1beta1/implicit_params_test.go
View file
Open in desktop

Large diffs are not rendered by default.

42 changes: 36 additions & 6 deletions pkg/apis/pipeline/v1beta1/param_context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,26 @@ package v1beta1
import (
"context"
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/config"
)

// paramCtxKey is the unique identifier for referencing param information from
// enableImplicitParamContextKeyType is the unique type for referencing whether
// Implicit Param behavior is enabled for the given request context.
// See [TEP-0023](https://github.com/tektoncd/community/blob/main/teps/0023-implicit-mapping.md).
//
// +k8s:openapi-gen=false
type enableImplicitParamContextKeyType struct{}

// paramCtxKey is the unique type for referencing param information from
// a context.Context. See [context.Context.Value](https://pkg.go.dev/context#Context)
// for more details.
var paramCtxKey struct{}
//
// +k8s:openapi-gen=false
type paramCtxKeyType struct{}

var (
enableImplicitParamContextKey = enableImplicitParamContextKeyType{}
paramCtxKey = paramCtxKeyType{}
)

// paramCtxVal is the data type stored in the param context.
// This maps param names -> ParamSpec.
Expand All @@ -37,7 +49,7 @@ func addContextParams(ctx context.Context, in []Param) context.Context {
return ctx
}

if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != "alpha" {
if !GetImplicitParamsEnabled(ctx) {
return ctx
}

Expand Down Expand Up @@ -73,7 +85,7 @@ func addContextParamSpec(ctx context.Context, in []ParamSpec) context.Context {
return ctx
}

if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != "alpha" {
if !GetImplicitParamsEnabled(ctx) {
return ctx
}

Expand Down Expand Up @@ -170,3 +182,21 @@ func getContextParamSpecs(ctx context.Context) []ParamSpec {
}
return out
}

// WithImplicitParamsEnabled enables implicit parameter behavior for the
// request context. If enabled, parameters will propagate down embedded request
// objects (e.g. PipelineRun -> Pipeline -> Task, Pipeline -> Task,
// TaskRun -> Task) during defaulting.
func WithImplicitParamsEnabled(ctx context.Context, v bool) context.Context {
return context.WithValue(ctx, enableImplicitParamContextKey, v)
}

// GetImplicitParamsEnabled returns whether implicit parameters are enabled for
// the given context.
func GetImplicitParamsEnabled(ctx context.Context) bool {
v := ctx.Value(enableImplicitParamContextKey)
if v == nil {
return false
}
return v.(bool)
}
25 changes: 8 additions & 17 deletions pkg/apis/pipeline/v1beta1/param_context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,20 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
)

func TestAddContextParams(t *testing.T) {
ctx := context.Background()

t.Run("no-alpha", func(t *testing.T) {
t.Run("not-enabled", func(t *testing.T) {
ctx := addContextParams(ctx, []Param{{Name: "a"}})
if v := ctx.Value(paramCtxKey); v != nil {
t.Errorf("expected no param context values, got %v", v)
}
})

// Enable Alpha features.
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags = &config.FeatureFlags{EnableAPIFields: "alpha"}
ctx = config.ToContext(ctx, cfg)
ctx = WithImplicitParamsEnabled(ctx, true)

// These test cases should run sequentially. Each step will modify the
// above context.
Expand Down Expand Up @@ -120,17 +117,15 @@ func TestAddContextParams(t *testing.T) {
func TestAddContextParamSpec(t *testing.T) {
ctx := context.Background()

t.Run("no-alpha", func(t *testing.T) {
t.Run("not-enabled", func(t *testing.T) {
ctx := addContextParamSpec(ctx, []ParamSpec{{Name: "a"}})
if v := ctx.Value(paramCtxKey); v != nil {
t.Errorf("expected no param context values, got %v", v)
}
})

// Enable Alpha features.
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags = &config.FeatureFlags{EnableAPIFields: "alpha"}
ctx = config.ToContext(ctx, cfg)
ctx = WithImplicitParamsEnabled(ctx, true)

// These test cases should run sequentially. Each step will modify the
// above context.
Expand Down Expand Up @@ -213,17 +208,15 @@ func TestGetContextParams(t *testing.T) {
Description: "racecar",
},
}
t.Run("no-alpha", func(t *testing.T) {
t.Run("not-enabled", func(t *testing.T) {
ctx := addContextParamSpec(ctx, want)
if v := getContextParamSpecs(ctx); v != nil {
t.Errorf("expected no param context values, got %v", v)
}
})

// Enable Alpha features.
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags = &config.FeatureFlags{EnableAPIFields: "alpha"}
ctx = config.ToContext(ctx, cfg)
ctx = WithImplicitParamsEnabled(ctx, true)

ctx = addContextParamSpec(ctx, want)

Expand Down Expand Up @@ -294,17 +287,15 @@ func TestGetContextParamSpecs(t *testing.T) {
Description: "racecar",
},
}
t.Run("no-alpha", func(t *testing.T) {
t.Run("not-enabled", func(t *testing.T) {
ctx := addContextParamSpec(ctx, want)
if v := getContextParamSpecs(ctx); v != nil {
t.Errorf("expected no param context values, got %v", v)
}
})

// Enable Alpha features.
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags = &config.FeatureFlags{EnableAPIFields: "alpha"}
ctx = config.ToContext(ctx, cfg)
ctx = WithImplicitParamsEnabled(ctx, true)

ctx = addContextParamSpec(ctx, want)
got := getContextParamSpecs(ctx)
Expand Down
7 changes: 3 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1beta1
import (
"context"

"github.com/tektoncd/pipeline/pkg/apis/config"
"knative.dev/pkg/apis"
)

Expand All @@ -35,7 +34,7 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
for i := range ps.Params {
ps.Params[i].SetDefaults(ctx)
}
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
if GetImplicitParamsEnabled(ctx) {
ctx = addContextParamSpec(ctx, ps.Params)
ps.Params = getContextParamSpecs(ctx)
}
Expand All @@ -50,7 +49,7 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
if pt.TaskSpec != nil {
// Only propagate param context to the spec - ref params should
// still be explicitly set.
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
if GetImplicitParamsEnabled(ctx) {
ctx = addContextParams(ctx, pt.Params)
ps.Tasks[i].Params = getContextParams(ctx, pt.Params...)
}
Expand All @@ -66,7 +65,7 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
}
}
if ft.TaskSpec != nil {
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
if GetImplicitParamsEnabled(ctx) {
ctx = addContextParams(ctx, ft.Params)
ps.Finally[i].Params = getContextParams(ctx, ft.Params...)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1beta1/pipelinerun_defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ func (pr *PipelineRun) SetDefaults(ctx context.Context) {

// SetDefaults implements apis.Defaultable
func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) {
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
ctx = WithImplicitParamsEnabled(ctx, true)
}

cfg := config.FromContextOrDefaults(ctx)
if prs.Timeout == nil && prs.Timeouts == nil {
prs.Timeout = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute}
Expand All @@ -52,7 +56,7 @@ func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) {
prs.PodTemplate = mergePodTemplateWithDefault(prs.PodTemplate, defaultPodTemplate)

if prs.PipelineSpec != nil {
if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" {
if GetImplicitParamsEnabled(ctx) {
ctx = addContextParams(ctx, prs.Params)
}
prs.PipelineSpec.SetDefaults(ctx)
Expand Down
170 changes: 3 additions & 167 deletions pkg/apis/pipeline/v1beta1/pipelinerun_defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ import (

func TestPipelineRunSpec_SetDefaults(t *testing.T) {
cases := []struct {
desc string
prs *v1beta1.PipelineRunSpec
want *v1beta1.PipelineRunSpec
ctxFn func(context.Context) context.Context
desc string
prs *v1beta1.PipelineRunSpec
want *v1beta1.PipelineRunSpec
}{
{
desc: "timeout is nil",
Expand Down Expand Up @@ -84,173 +83,10 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
},
},
},
{
desc: "implicit params",
ctxFn: func(ctx context.Context) context.Context {
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags = &config.FeatureFlags{EnableAPIFields: "alpha"}
return config.ToContext(ctx, cfg)
},
prs: &v1beta1.PipelineRunSpec{
Params: []v1beta1.Param{
{
Name: "foo",
Value: v1beta1.ArrayOrString{
StringVal: "a",
},
},
{
Name: "bar",
Value: v1beta1.ArrayOrString{
ArrayVal: []string{"b"},
},
},
},
PipelineSpec: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{
{
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{},
},
},
{
TaskRef: &v1beta1.TaskRef{
Name: "baz",
},
},
},
Finally: []v1beta1.PipelineTask{
{
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{},
},
},
{
TaskRef: &v1beta1.TaskRef{
Name: "baz",
},
},
},
},
},
want: &v1beta1.PipelineRunSpec{
ServiceAccountName: config.DefaultServiceAccountValue,
Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
Params: []v1beta1.Param{
{
Name: "foo",
Value: v1beta1.ArrayOrString{
StringVal: "a",
},
},
{
Name: "bar",
Value: v1beta1.ArrayOrString{
ArrayVal: []string{"b"},
},
},
},
PipelineSpec: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{
{
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{
{
Name: "foo",
Type: v1beta1.ParamTypeString,
},
{
Name: "bar",
Type: v1beta1.ParamTypeArray,
},
},
},
},
Params: []v1beta1.Param{
{
Name: "foo",
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeString,
StringVal: "$(params.foo)",
},
},
{
Name: "bar",
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeArray,
ArrayVal: []string{"$(params.bar[*])"},
},
},
},
},
{
TaskRef: &v1beta1.TaskRef{
Name: "baz",
Kind: v1beta1.NamespacedTaskKind,
},
},
},
Finally: []v1beta1.PipelineTask{
{
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{
{
Name: "foo",
Type: v1beta1.ParamTypeString,
},
{
Name: "bar",
Type: v1beta1.ParamTypeArray,
},
},
},
},
Params: []v1beta1.Param{
{
Name: "foo",
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeString,
StringVal: "$(params.foo)",
},
},
{
Name: "bar",
Value: v1beta1.ArrayOrString{
Type: v1beta1.ParamTypeArray,
ArrayVal: []string{"$(params.bar[*])"},
},
},
},
},
{
TaskRef: &v1beta1.TaskRef{
Name: "baz",
Kind: v1beta1.NamespacedTaskKind,
},
},
},
Params: []v1beta1.ParamSpec{
{
Name: "foo",
Type: v1beta1.ParamTypeString,
},
{
Name: "bar",
Type: v1beta1.ParamTypeArray,
},
},
},
},
},
}
for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
ctx := context.Background()
if tc.ctxFn != nil {
ctx = tc.ctxFn(ctx)
}
tc.prs.SetDefaults(ctx)

sortParamSpecs := func(x, y v1beta1.ParamSpec) bool {
Expand Down
Loading