Skip to content

Commit

Permalink
[WIP] Decouple v1beta1 beta feature versioning
Browse files Browse the repository at this point in the history
  • Loading branch information
JeromeJu committed Jul 18, 2023
1 parent 2250e40 commit c746eac
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 10 deletions.
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
tests := []struct {
name string
tasks PipelineTask
enableAPIFields bool
enableAPIFields string
enableBundles bool
}{{
name: "pipeline task - valid taskRef name",
Expand All @@ -240,11 +240,13 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
tasks: PipelineTask{
TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}},
},
enableAPIFields: "beta",
}, {
name: "pipeline task - use of params",
tasks: PipelineTask{
TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{}}},
},
enableAPIFields: "beta",
}, {
name: "pipeline task - use of bundle with the feature flag set",
tasks: PipelineTask{
Expand All @@ -259,9 +261,7 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
cfg := &config.Config{
FeatureFlags: &config.FeatureFlags{},
}
if tt.enableAPIFields {
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
}
cfg.FeatureFlags.EnableAPIFields = tt.enableAPIFields
if tt.enableBundles {
cfg.FeatureFlags.EnableTektonOCIBundles = true
}
Expand Down
54 changes: 54 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,60 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
return errs
}

// ValidateBetaFields returns an error if the Pipeline spec uses beta features but does not
// have "enable-api-fields" set to "alpha" or "beta".
func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
var errs *apis.FieldError
// Object parameters
for i, p := range ps.Params {
if p.Type == ParamTypeObject {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
}
}
// Indexing into array parameters
arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
if len(arrayParamIndexingRefs) != 0 && !config.CheckAlphaOrBetaAPIFields(ctx) {
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayParamIndexingRefs)))
}
// array and object results
for i, result := range ps.Results {
switch result.Type {
case ResultsTypeObject:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
case ResultsTypeArray:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
case ResultsTypeString:
default:
}
}
for i, pt := range ps.Tasks {
errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i))
}
for i, pt := range ps.Finally {
errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i))
}

return errs
}

// validateBetaFields returns an error if the PipelineTask uses beta features but does not
// have "enable-api-fields" set to "alpha" or "beta".
func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError {
var errs *apis.FieldError
if pt.TaskRef != nil {
// Resolvers
if pt.TaskRef.Resolver != "" {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
}
if len(pt.TaskRef.Params) > 0 {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
}
} else if pt.TaskSpec != nil {
errs = errs.Also(pt.TaskSpec.ValidateBetaFields(ctx))
}
return errs
}

// ValidatePipelineTasks ensures that pipeline tasks has unique label, pipeline tasks has specified one of
// taskRef or taskSpec, and in case of a pipeline task with taskRef, it has a reference to a valid task (task name)
func ValidatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks []PipelineTask) *apis.FieldError {
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,35 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
return errs
}

// ValidateBetaFields returns an error if the Task spec uses beta features but does not
// have "enable-api-fields" set to "alpha" or "beta".
func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
var errs *apis.FieldError
// Object parameters
for i, p := range ts.Params {
if p.Type == ParamTypeObject {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
}
}
// Indexing into array parameters
arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams()
if len(arrayIndexParamRefs) != 0 && !config.CheckAlphaOrBetaAPIFields(ctx) {
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayIndexParamRefs)))
}
// Array and object results
for i, result := range ts.Results {
switch result.Type {
case ResultsTypeObject:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
case ResultsTypeArray:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
case ResultsTypeString:
default:
}
}
return errs
}

// ValidateUsageOfDeclaredParameters validates that all parameters referenced in the Task are declared by the Task.
func ValidateUsageOfDeclaredParameters(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError {
var errs *apis.FieldError
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1beta1/taskref_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"strings"

"github.com/google/go-containerregistry/pkg/name"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/version"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
)
Expand All @@ -35,6 +37,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
switch {
case ref.Resolver != "" || ref.Params != nil:
if ref.Resolver != "" {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
if ref.Name != "" {
errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
}
Expand All @@ -43,6 +46,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
}
}
if ref.Params != nil {
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
if ref.Name != "" {
errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelineref.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ func readRuntimeObjectAsPipeline(ctx context.Context, obj runtime.Object, k8s ku
case *v1beta1.Pipeline:
// Verify the Pipeline once we fetch from the remote resolution, mutating, validation and conversion of the pipeline should happen after the verification, since signatures are based on the remote pipeline contents
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
if err := obj.Spec.ValidateBetaFields(ctx); err != nil {
return nil, nil, fmt.Errorf("invalid Pipeline %s: %w", obj.GetName(), err)
}
p := &v1.Pipeline{
TypeMeta: metav1.TypeMeta{
Kind: "Pipeline",
Expand Down
27 changes: 21 additions & 6 deletions pkg/reconciler/pipelinerun/resources/pipelineref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,11 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
pipelineRef := &v1.PipelineRef{ResolverRef: v1.ResolverRef{Resolver: "git"}}

testcases := []struct {
name string
pipelineYAML string
wantPipeline *v1.Pipeline
wantErr bool
name string
pipelineYAML string
enableAPIFields string
wantPipeline *v1.Pipeline
wantErr bool
}{{
name: "v1beta1 pipeline",
pipelineYAML: strings.Join([]string{
Expand All @@ -286,13 +287,25 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
}, "\n"),
wantPipeline: parse.MustParseV1Pipeline(t, pipelineYAMLString),
}, {
name: "v1beta1 pipeline with beta features",
name: "v1beta1 pipeline with beta features enableAPIFields beta",
pipelineYAML: strings.Join([]string{
"kind: Pipeline",
"apiVersion: tekton.dev/v1beta1",
pipelineYAMLStringWithBetaFeatures,
}, "\n"),
enableAPIFields: "beta",
wantPipeline: parse.MustParseV1Pipeline(t, pipelineYAMLStringWithBetaFeatures),
wantErr: true,
}, {
name: "v1beta1 pipeline with beta features enableAPIFields stable",
pipelineYAML: strings.Join([]string{
"kind: Pipeline",
"apiVersion: tekton.dev/v1beta1",
pipelineYAMLStringWithBetaFeatures,
}, "\n"),
wantPipeline: parse.MustParseV1Pipeline(t, pipelineYAMLStringWithBetaFeatures),
enableAPIFields: "stable",
wantPipeline: parse.MustParseV1Pipeline(t, pipelineYAMLStringWithBetaFeatures),
wantErr: true,
}, {
name: "v1 pipeline",
pipelineYAML: strings.Join([]string{
Expand Down Expand Up @@ -323,6 +336,8 @@ func TestGetPipelineFunc_RemoteResolution(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
resolved := test.NewResolvedResource([]byte(tc.pipelineYAML), nil /* annotations */, sampleRefSource.DeepCopy(), nil /* data error */)
requester := test.NewRequester(resolved, nil)
cfg.FeatureFlags.EnableAPIFields = tc.enableAPIFields

fn := resources.GetPipelineFunc(ctx, nil, nil, requester, &v1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
Spec: v1.PipelineRunSpec{
Expand Down
3 changes: 3 additions & 0 deletions pkg/reconciler/taskrun/resources/taskref.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ func readRuntimeObjectAsTask(ctx context.Context, obj runtime.Object, k8s kubern
case *v1beta1.Task:
// Verify the Task once we fetch from the remote resolution, mutating, validation and conversion of the task should happen after the verification, since signatures are based on the remote task contents
vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies)
if err := obj.Spec.ValidateBetaFields(ctx); err != nil {
return nil, nil, fmt.Errorf("invalid Task %s: %w", obj.GetName(), err)
}
t := &v1.Task{
TypeMeta: metav1.TypeMeta{
Kind: "Task",
Expand Down

0 comments on commit c746eac

Please sign in to comment.