Skip to content

Commit

Permalink
array results and indexing into array promoted to beta
Browse files Browse the repository at this point in the history
Pipelines 0.38 introduced two alpha features - producing array results
and indexing into array param. This commit is promoting these two features
to beta such that these features can be used by the task authors and pipeline
authors by setting enable-api-fields to either beta or alpha.

These features are still not availalbe in stable API i.e. these features
will not work with enable-api-fields set to stable

Signed-off-by: pritidesai <pdesai@us.ibm.com>
  • Loading branch information
pritidesai committed Feb 7, 2023
1 parent 1dffa32 commit 9f7716c
Show file tree
Hide file tree
Showing 16 changed files with 102 additions and 49 deletions.
18 changes: 9 additions & 9 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -942,17 +942,17 @@ Sharing `Results` between `Tasks` in a `Pipeline` happens via
a `Result` and another receives it as a `Parameter` with a variable such as
`$(tasks.<task-name>.results.<result-name>)`. Pipeline support two new types of
results and parameters: array `[]string` and object `map[string]string`.
Both are alpha features and can be enabled by setting `enable-api-fields` to `alpha`.
Object result is alpha feature and can be enabled by setting `enable-api-fields` to `alpha`.

| Result Type | Parameter Type | Specification | `enable-api-fields` |
|-------------|----------------|--------------------------------------------------|-------------------|
| string | string | `$(tasks.<task-name>.results.<result-name>)` | stable |
| array | array | `$(tasks.<task-name>.results.<result-name>[*])` | alpha |
| array | string | `$(tasks.<task-name>.results.<result-name>[i])` | alpha |
| object | object | `$(tasks.<task-name>.results.<result-name>[*])` | alpha |
| object | string | `$(tasks.<task-name>.results.<result-name>.key)` | alpha |
|-------------|----------------|--------------------------------------------------|---------------------|
| string | string | `$(tasks.<task-name>.results.<result-name>)` | stable |
| array | array | `$(tasks.<task-name>.results.<result-name>[*])` | beta |
| array | string | `$(tasks.<task-name>.results.<result-name>[i])` | beta |
| object | object | `$(tasks.<task-name>.results.<result-name>[*])` | alpha |
| object | string | `$(tasks.<task-name>.results.<result-name>.key)` | alpha |

**Note:** Whole Array and Object `Results` (using star notation) cannot be referred in `script` and `args`.
**Note:** Whole Array and Object `Results` (using star notation) cannot be referred in `script`.

**Note:** `Matrix` does not support `object` and `array` results.

Expand Down Expand Up @@ -1039,7 +1039,7 @@ results:

For an end-to-end example, see [`Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/pipelinerun-results.yaml).

Array and object results is supported as alpha feature, see [`Array Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml).
Object result is supported as alpha feature, see [`Array and Object Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml).

```yaml
results:
Expand Down
2 changes: 1 addition & 1 deletion docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ or [at the `Pipeline` level](./pipelines.md#emitting-results-from-a-pipeline).
#### Emitting Array `Results`

Tekton Task also supports defining a result of type `array` and `object` in addition to `string`.
Emitting a task result of type `array` is an `alpha` feature implemented based on the
Emitting a task result of type `array` is implemented based on the
[TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md#emitting-array-results).
You can initialize `array` results from a `task` using JSON escaped string, for example, to assign the following
list of animals to an array result:
Expand Down
11 changes: 11 additions & 0 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,11 @@ func EnableBetaAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, "beta")
}

// EnableStableAPIFields enables stable features in an existing context (for use in testing)
func EnableStableAPIFields(ctx context.Context) context.Context {
return setEnableAPIFields(ctx, StableAPIFields)
}

// CheckEnforceResourceVerificationMode returns true if the ResourceVerificationMode is EnforceResourceVerificationMode
// else returns false
func CheckEnforceResourceVerificationMode(ctx context.Context) bool {
Expand All @@ -337,6 +342,12 @@ func CheckWarnResourceVerificationMode(ctx context.Context) bool {
return cfg.FeatureFlags.ResourceVerificationMode == WarnResourceVerificationMode
}

// CheckAlphaOrBetaAPIFields return true if the enable-api-fields is either set to alpha or set to beta
func CheckAlphaOrBetaAPIFields(ctx context.Context) bool {
cfg := FromContextOrDefaults(ctx)
return cfg.FeatureFlags.EnableAPIFields == AlphaAPIFields || cfg.FeatureFlags.EnableAPIFields == BetaAPIFields
}

func setEnableAPIFields(ctx context.Context, want string) context.Context {
featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": want,
Expand Down
31 changes: 31 additions & 0 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,37 @@ func TestCheckWarnResourceVerificationMode(t *testing.T) {
}
}

// TestCheckAlphaOrBetaAPIFields validates CheckAlphaOrBetaAPIFields for alpha, beta, and stable context
func TestCheckAlphaOrBetaAPIFields(t *testing.T) {
ctx := context.Background()
type testCase struct {
name string
c context.Context
expectedResult bool
}
testCases := []testCase{{
name: "when enable-api-fields is set to alpha",
c: config.EnableAlphaAPIFields(ctx),
expectedResult: true,
}, {
name: "when enable-api-fields is set to beta",
c: config.EnableBetaAPIFields(ctx),
expectedResult: true,
}, {
name: "when enable-api-fields is set to stable",
c: config.EnableStableAPIFields(ctx),
expectedResult: false,
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
e := config.CheckAlphaOrBetaAPIFields(tc.c)
if (tc.expectedResult && !e) || (!tc.expectedResult && e) {
t.Errorf("Failed to validate CheckAlphaOrBetaAPIFields for \"%v\", expected \"%v\" but got \"%v\"", tc.name, tc.expectedResult, e)
}
})
}
}

func verifyConfigFileWithExpectedFeatureFlagsConfig(t *testing.T, fileName string, expectedConfig *config.FeatureFlags) {
t.Helper()
cm := test.ConfigMapFromTestFile(t, fileName)
Expand Down
20 changes: 11 additions & 9 deletions pkg/apis/pipeline/v1/result_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,23 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) {
if !resultNameFormatRegex.MatchString(tr.Name) {
return apis.ErrInvalidKeyName(tr.Name, "name", fmt.Sprintf("Name must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", ResultNameFormat))
}
// Array and Object are alpha features
if tr.Type == ResultsTypeArray || tr.Type == ResultsTypeObject {

switch {
// Object are alpha features
case tr.Type == ResultsTypeObject:
errs := validateObjectResult(tr)
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields))
return errs
}

// Array results is a beta feature - check if the feature flag is set to "beta" or "alpha"
case tr.Type == ResultsTypeArray:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
return errs
// Resources created before the result. Type was introduced may not have Type set
// and should be considered valid
if tr.Type == "" {
case tr.Type == "":
return nil
}

// By default the result type is string
if tr.Type != ResultsTypeString {
// By default, the result type is string
case tr.Type != ResultsTypeString:
return apis.ErrInvalidValue(tr.Type, "type", "type must be string")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/result_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func TestResultsValidateError(t *testing.T) {
},
apiFields: "stable",
expectedError: apis.FieldError{
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"",
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
},
}, {
name: "invalid object result type in stable",
Expand Down
20 changes: 11 additions & 9 deletions pkg/apis/pipeline/v1beta1/result_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,23 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) {
if !resultNameFormatRegex.MatchString(tr.Name) {
return apis.ErrInvalidKeyName(tr.Name, "name", fmt.Sprintf("Name must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", ResultNameFormat))
}
// Array and Object is alpha feature
if tr.Type == ResultsTypeArray || tr.Type == ResultsTypeObject {

switch {
// Object are alpha features
case tr.Type == ResultsTypeObject:
errs := validateObjectResult(tr)
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields))
return errs
}

// Array results is a beta feature - make sure the feature flag is set to "beta"
case tr.Type == ResultsTypeArray:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
return errs
// Resources created before the result. Type was introduced may not have Type set
// and should be considered valid
if tr.Type == "" {
case tr.Type == "":
return nil
}

// By default the result type is string
if tr.Type != ResultsTypeString {
// By default, the result type is string
case tr.Type != ResultsTypeString:
return apis.ErrInvalidValue(tr.Type, "type", "type must be string")
}

Expand Down
14 changes: 13 additions & 1 deletion pkg/apis/pipeline/v1beta1/result_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ func TestResultsValidate(t *testing.T) {
},

apiFields: "alpha",
}, {
name: "valid result type array",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeArray,
Description: "my great result",
},

apiFields: "beta",
}, {
name: "valid result type object",
Result: v1beta1.TaskResult{
Expand All @@ -74,6 +83,9 @@ func TestResultsValidate(t *testing.T) {
if tt.apiFields == "alpha" {
ctx = config.EnableAlphaAPIFields(ctx)
}
if tt.apiFields == config.BetaAPIFields {
ctx = config.EnableBetaAPIFields(ctx)
}
if err := tt.Result.Validate(ctx); err != nil {
t.Errorf("TaskSpec.Validate() = %v", err)
}
Expand Down Expand Up @@ -122,7 +134,7 @@ func TestResultsValidateError(t *testing.T) {
},
apiFields: "stable",
expectedError: apis.FieldError{
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"",
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
},
}, {
name: "invalid object result type in stable",
Expand Down
10 changes: 4 additions & 6 deletions pkg/reconciler/pipelinerun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,15 @@ func ApplyParameters(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.P
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}
objectReplacements := map[string]map[string]string{}
cfg := config.FromContextOrDefaults(ctx)

// Set all the default stringReplacements
for _, p := range p.Params {
if p.Default != nil {
switch p.Default.Type {
case v1beta1.ParamTypeArray:
for _, pattern := range paramPatterns {
// array indexing for param is alpha feature
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
// array indexing for param is beta feature - the feature flag can be either set to alpha or beta
if config.CheckAlphaOrBetaAPIFields(ctx) {
for i := 0; i < len(p.Default.ArrayVal); i++ {
stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Default.ArrayVal[i]
}
Expand Down Expand Up @@ -108,14 +107,13 @@ func paramsFromPipelineRun(ctx context.Context, pr *v1beta1.PipelineRun) (map[st
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}
objectReplacements := map[string]map[string]string{}
cfg := config.FromContextOrDefaults(ctx)

for _, p := range pr.Spec.Params {
switch p.Value.Type {
case v1beta1.ParamTypeArray:
for _, pattern := range paramPatterns {
// array indexing for param is alpha feature
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
// array indexing for param is beta feature - the feature flag can be either set to alpha ot beta
if config.CheckAlphaOrBetaAPIFields(ctx) {
for i := 0; i < len(p.Value.ArrayVal); i++ {
stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Value.ArrayVal[i]
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1728,7 +1728,7 @@ func TestApplyParameters(t *testing.T) {
func TestApplyParameters_ArrayIndexing(t *testing.T) {
ctx := context.Background()
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
ctx = config.ToContext(ctx, cfg)
for _, tt := range []struct {
name string
Expand Down
5 changes: 2 additions & 3 deletions pkg/reconciler/pipelinerun/resources/validate_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,9 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip
return nil
}

// ValidateParamArrayIndex validate if the array indexing param reference target is existent
// ValidateParamArrayIndex validate if the array indexing param reference target is existent
func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error {
cfg := config.FromContextOrDefaults(ctx)
if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields {
if !config.CheckAlphaOrBetaAPIFields(ctx) {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/resources/validate_params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func TestValidateObjectParamRequiredKeys_Valid(t *testing.T) {
func TestValidateParamArrayIndex_valid(t *testing.T) {
ctx := context.Background()
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
ctx = config.ToContext(ctx, cfg)
for _, tt := range []struct {
name string
Expand Down Expand Up @@ -508,7 +508,7 @@ func TestValidateParamArrayIndex_valid(t *testing.T) {
func TestValidateParamArrayIndex_invalid(t *testing.T) {
ctx := context.Background()
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
ctx = config.ToContext(ctx, cfg)
for _, tt := range []struct {
name string
Expand Down
10 changes: 4 additions & 6 deletions pkg/reconciler/taskrun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,15 @@ func ApplyParameters(ctx context.Context, spec *v1beta1.TaskSpec, tr *v1beta1.Ta
// that need to be further processed.
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}
cfg := config.FromContextOrDefaults(ctx)

// Set all the default stringReplacements
for _, p := range defaults {
if p.Default != nil {
switch p.Default.Type {
case v1beta1.ParamTypeArray:
for _, pattern := range paramPatterns {
// array indexing for param is alpha feature
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
// array indexing for param is beta feature
if config.CheckAlphaOrBetaAPIFields(ctx) {
for i := 0; i < len(p.Default.ArrayVal); i++ {
stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Default.ArrayVal[i]
}
Expand Down Expand Up @@ -100,14 +99,13 @@ func paramsFromTaskRun(ctx context.Context, tr *v1beta1.TaskRun) (map[string]str
// that need to be further processed.
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}
cfg := config.FromContextOrDefaults(ctx)

for _, p := range tr.Spec.Params {
switch p.Value.Type {
case v1beta1.ParamTypeArray:
for _, pattern := range paramPatterns {
// array indexing for param is alpha feature
if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields {
// array indexing for param is beta feature
if config.CheckAlphaOrBetaAPIFields(ctx) {
for i := 0; i < len(p.Value.ArrayVal); i++ {
stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Value.ArrayVal[i]
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/taskrun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ func TestApplyParameters_ArrayIndexing(t *testing.T) {
})
ctx := context.Background()
cfg := config.FromContextOrDefaults(ctx)
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
ctx = config.ToContext(ctx, cfg)
got := resources.ApplyParameters(ctx, simpleTaskSpecArrayIndexing, tr, dp...)
if d := cmp.Diff(want, got); d != "" {
Expand Down

0 comments on commit 9f7716c

Please sign in to comment.