Skip to content

Commit

Permalink
ignore enable-tekton-oci-bundles feature flag
Browse files Browse the repository at this point in the history
Signed-off-by: Jeeva Kandasamy <jkandasa@redhat.com>
  • Loading branch information
jkandasa authored and tekton-robot committed Sep 27, 2024
1 parent 5462fd7 commit d07eadb
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 24 deletions.
2 changes: 0 additions & 2 deletions docs/TektonConfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ The TektonConfig CR provides the following features
enable-param-enum: false
enable-provenance-in-status: true
enable-step-actions: false
enable-tekton-oci-bundles: false
enforce-nonfalsifiability: none
keep-pod-on-cancel: false
max-result-size: 4096
Expand Down Expand Up @@ -218,7 +217,6 @@ pipeline:
disable-working-directory-overwrite: true
enable-api-fields: stable
enable-custom-tasks: false
enable-tekton-oci-bundles: false
metrics.pipelinerun.duration-type: histogram
metrics.pipelinerun.level: pipelinerun
metrics.taskrun.duration-type: histogram
Expand Down
7 changes: 0 additions & 7 deletions docs/TektonPipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ spec:
enable-param-enum: false
enable-provenance-in-status: true
enable-step-actions: false
enable-tekton-oci-bundles: false
enforce-nonfalsifiability: none
keep-pod-on-cancel: false
max-result-size: 4096
Expand Down Expand Up @@ -113,12 +112,6 @@ injected sidecars, setting this option to false can lead to unexpected behavior.
See more info [here](https://github.com/tektoncd/pipeline/issues/2981).


- `enable-tekton-oci-bundles` (Default: `false`)

Setting this flag to "true" enables the use of Tekton OCI bundle. This is an experimental feature and thus should
still be considered an alpha feature.


- `enable-custom-tasks` (Default: `false`)

Setting this flag to "true" enables the use of custom tasks from within pipelines. This is an experimental feature
Expand Down
3 changes: 1 addition & 2 deletions pkg/apis/operator/v1alpha1/tektonconfig_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ func Test_SetDefaults_Pipeline_Properties(t *testing.T) {
}

tc.SetDefaults(context.TODO())
if *tc.Spec.Pipeline.SendCloudEventsForRuns != true ||
*tc.Spec.Pipeline.EnableTektonOciBundles != false {
if *tc.Spec.Pipeline.SendCloudEventsForRuns != true {
t.Error("Setting default failed for TektonConfig (spec.pipeline.pipelineProperties)")
}
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/apis/operator/v1alpha1/tektonpipeline_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ func (p *Pipeline) setDefaults() {
if p.RequireGitSshSecretKnownHosts == nil {
p.RequireGitSshSecretKnownHosts = ptr.Bool(config.DefaultRequireGitSSHSecretKnownHosts)
}
if p.EnableTektonOciBundles == nil {
p.EnableTektonOciBundles = ptr.Bool(config.DefaultEnableTektonOciBundles)
}

// not in use, see: https://github.com/tektoncd/pipeline/pull/7789
// this field is removed from pipeline component
// keeping here to maintain the API compatibility
p.EnableTektonOciBundles = nil

if p.EnableCustomTasks == nil {
// EnableCustomTask is always enable
p.EnableCustomTasks = ptr.Bool(true)
Expand Down
36 changes: 35 additions & 1 deletion pkg/apis/operator/v1alpha1/tektonpipeline_defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/test/diff"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -45,7 +46,6 @@ func Test_SetDefaults_PipelineProperties(t *testing.T) {
AwaitSidecarReadiness: ptr.Bool(true),
RunningInEnvironmentWithInjectedSidecars: ptr.Bool(true),
RequireGitSshSecretKnownHosts: ptr.Bool(false),
EnableTektonOciBundles: ptr.Bool(false),
EnableCustomTasks: ptr.Bool(true),
EnableApiFields: "beta",
EmbeddedStatus: "",
Expand Down Expand Up @@ -84,3 +84,37 @@ func Test_SetDefaults_PipelineProperties(t *testing.T) {
t.Errorf("failed to update deployment %s", diff.PrintWantGot(d))
}
}

// not in use, see: https://github.com/tektoncd/pipeline/pull/7789
// this field is removed from pipeline component
// keeping in types to maintain the API compatibility
// this test verifies that, "EnableTektonOciBundles" always keeps nil on defaults
func TestEnableTektonOciBundlesIgnored(t *testing.T) {
tp := &TektonPipeline{
Spec: TektonPipelineSpec{
Pipeline: Pipeline{
PipelineProperties: PipelineProperties{
EnableTektonOciBundles: ptr.Bool(true),
},
},
},
}
ctx := context.TODO()

tests := []struct {
name string
enableTektonOciBundles *bool
}{
{name: "with-true", enableTektonOciBundles: ptr.Bool(true)},
{name: "with-false", enableTektonOciBundles: ptr.Bool(false)},
{name: "with-nil", enableTektonOciBundles: nil},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tp.Spec.Pipeline.EnableTektonOciBundles = test.enableTektonOciBundles
tp.SetDefaults(ctx)
assert.Nil(t, tp.Spec.Pipeline.EnableTektonOciBundles, "EnableTektonOciBundles removed from pipeline and should be nil on defaulting")
})
}
}
7 changes: 6 additions & 1 deletion pkg/apis/operator/v1alpha1/tektonpipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ type PipelineProperties struct {
AwaitSidecarReadiness *bool `json:"await-sidecar-readiness,omitempty"`
RunningInEnvironmentWithInjectedSidecars *bool `json:"running-in-environment-with-injected-sidecars,omitempty"`
RequireGitSshSecretKnownHosts *bool `json:"require-git-ssh-secret-known-hosts,omitempty"`
EnableTektonOciBundles *bool `json:"enable-tekton-oci-bundles,omitempty"`
EnableCustomTasks *bool `json:"enable-custom-tasks,omitempty"`
EnableApiFields string `json:"enable-api-fields,omitempty"`
EmbeddedStatus string `json:"embedded-status,omitempty"`
Expand All @@ -102,6 +101,12 @@ type PipelineProperties struct {
// This field will be removed, see https://github.com/tektoncd/operator/issues/1497
// originally this field was removed in https://github.com/tektoncd/operator/pull/1481
// there is no use with this field, just adding back to unblock the upgrade

// not in use, see: https://github.com/tektoncd/pipeline/pull/7789
// this field is removed from pipeline component
// keeping here to maintain the API compatibility
EnableTektonOciBundles *bool `json:"enable-tekton-oci-bundles,omitempty"`

VerificationMode string `json:"verification-mode,omitempty"`
VerificationNoMatchPolicy string `json:"trusted-resources-verification-no-match-policy,omitempty"`
EnableProvenanceInStatus *bool `json:"enable-provenance-in-status,omitempty"`
Expand Down
10 changes: 5 additions & 5 deletions pkg/apis/operator/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions pkg/reconciler/common/transformers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,7 @@ func TestAddConfigMapValues_PipelineProperties(t *testing.T) {
assertNoEror(t, err)

prop := v1alpha1.PipelineProperties{
EnableTektonOciBundles: ptr.Bool(true),
EnableApiFields: "stable",
EnableApiFields: "stable",
}

manifest, err = manifest.Transform(AddConfigMapValues("test1", prop))
Expand All @@ -526,7 +525,6 @@ func TestAddConfigMapValues_PipelineProperties(t *testing.T) {
assertNoEror(t, err)

assert.Equal(t, cm.Data["foo"], "bar")
assert.Equal(t, cm.Data["enable-tekton-oci-bundles"], "true")
assert.Equal(t, cm.Data["enable-api-fields"], "stable")
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: feature-flags
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
disable-affinity-assistant: "false"
coschedule: "workspaces"
disable-creds-init: "false"
await-sidecar-readiness: "true"
running-in-environment-with-injected-sidecars: "true"
require-git-ssh-secret-known-hosts: "false"
enable-tekton-oci-bundles: "false"
enable-api-fields: "beta"
send-cloudevents-for-runs: "false"
trusted-resources-verification-no-match-policy: "ignore"
enable-provenance-in-status: "true"
enforce-nonfalsifiability: "none"
results-from: "termination-message"
set-security-context: "false"
keep-pod-on-cancel: "false"
enable-cel-in-whenexpression: "false"
enable-step-actions: "false"
enable-artifacts: "false"
enable-param-enum: "false"
disable-inline-spec: ""
enable-concise-resolver-syntax: "false"
enable-kubernetes-sidecar: "false"
5 changes: 5 additions & 0 deletions pkg/reconciler/kubernetes/tektonpipeline/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ func filterAndTransform(extension common.Extension) client.FilterAndTransform {
return func(ctx context.Context, manifest *mf.Manifest, comp v1alpha1.TektonComponent) (*mf.Manifest, error) {
pipeline := comp.(*v1alpha1.TektonPipeline)

// not in use, see: https://github.com/tektoncd/pipeline/pull/7789
// this field is removed from pipeline component
// still keeping types to maintain the API compatibility
pipeline.Spec.Pipeline.EnableTektonOciBundles = nil

images := common.ToLowerCaseKeys(common.ImagesFromEnv(common.PipelinesImagePrefix))
instance := comp.(*v1alpha1.TektonPipeline)
// adding extension's transformers first to run them before `extra` transformers
Expand Down
54 changes: 54 additions & 0 deletions pkg/reconciler/kubernetes/tektonpipeline/transform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ limitations under the License.
package tektonpipeline

import (
"context"
"encoding/json"
"fmt"
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
"github.com/tektoncd/operator/pkg/reconciler/common"
"github.com/tektoncd/pipeline/test/diff"
"gotest.tools/v3/assert"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -365,3 +367,55 @@ func TestUpdateResolverConfigEnvironmentsInDeployment(t *testing.T) {
})
}
}

// not in use, see: https://github.com/tektoncd/pipeline/pull/7789
// this field is removed from pipeline component
// keeping in types to maintain the API compatibility
// this test verifies that, "EnableTektonOciBundles" always not present in the feature flags config map
func TestEnableTektonOciBundlesFeatureFlag(t *testing.T) {
tp := &v1alpha1.TektonPipeline{
Spec: v1alpha1.TektonPipelineSpec{
Pipeline: v1alpha1.Pipeline{
PipelineProperties: v1alpha1.PipelineProperties{
EnableTektonOciBundles: ptr.Bool(true),
},
},
},
}
ctx := context.TODO()

tests := []struct {
name string
enableTektonOciBundles *bool
expectedValue string
}{
{name: "with-true", enableTektonOciBundles: ptr.Bool(true), expectedValue: "false"},
{name: "with-false", enableTektonOciBundles: ptr.Bool(false), expectedValue: "false"},
{name: "with-nil", enableTektonOciBundles: nil, expectedValue: "false"},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tp.Spec.Pipeline.EnableTektonOciBundles = test.enableTektonOciBundles

// get manifests
manifest, err := common.Fetch("./testdata/tektonpipeline-feature-flags-base.yaml")
assert.NilError(t, err, "error on fetching testdata")

transformers := filterAndTransform(common.NoExtension(ctx))
_, err = transformers(ctx, &manifest, tp)
assert.NilError(t, err)

resources := manifest.Resources()
assert.Assert(t, len(resources) > 0)

featureFlagsMap := corev1.ConfigMap{}
err = apimachineryRuntime.DefaultUnstructuredConverter.FromUnstructured(resources[0].Object, &featureFlagsMap)
assert.NilError(t, err)

flagValue, found := featureFlagsMap.Data["enable-tekton-oci-bundles"]
assert.Assert(t, found == true, "'enable-tekton-oci-bundles' not found")
assert.Assert(t, flagValue == test.expectedValue, "'enable-tekton-oci-bundles' is not '%s'", test.expectedValue)
})
}
}

0 comments on commit d07eadb

Please sign in to comment.