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

[main] ignore enable-tekton-oci-bundles feature flag #2344

Merged
merged 1 commit into from
Sep 26, 2024
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
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"`
Copy link
Member

Choose a reason for hiding this comment

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

So in the next or later release we can directly remove this field right from the API, since we are setting this to nil in defaulting ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@PuneetPunamiya we never remove this from v1alpha1 API


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 @@ -515,8 +515,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 @@ -527,7 +526,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
Comment on lines +371 to +373
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need this comment here

// 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)
})
}
}