From b53e0d80b8eb25c065d36fd716511187163a05d4 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 30 Aug 2022 10:53:24 -0400 Subject: [PATCH] Add combined remote resolvers binary Part of #4710 This adds the `hub`, `git`, and `bundles` resolvers. The individual resolvers can be enabled via the `enable-hub-resolver`, `enable-git-resolver`, and `enable-bundles-resolver` feature flags. Signed-off-by: Andrew Bayer --- cmd/resolvers/kodata/HEAD | 1 + cmd/resolvers/kodata/LICENSE | 1 + cmd/resolvers/kodata/refs | 1 + cmd/resolvers/kodata/third_party | 1 + cmd/resolvers/main.go | 37 +++++++++++ docs/bundle-resolver.md | 4 +- docs/install.md | 2 +- docs/resolution-getting-started.md | 2 +- docs/resolution.md | 2 +- pkg/apis/config/feature_flags.go | 26 ++++++++ pkg/apis/config/feature_flags_test.go | 2 + pkg/resolution/resolver/bundle/resolver.go | 19 ++++++ .../resolver/bundle/resolver_test.go | 57 +++++++++++++++-- .../resolver/framework/configstore.go | 27 +++++++- .../resolver/framework/controller.go | 13 +--- .../framework/testing/fakecontroller.go | 54 ++++++++++++++++ .../resolver/framework/testing/featureflag.go | 49 +++++++++++++++ pkg/resolution/resolver/git/resolver.go | 22 ++++++- pkg/resolution/resolver/git/resolver_test.go | 58 ++++++++++++++++-- pkg/resolution/resolver/hub/resolver.go | 19 ++++++ pkg/resolution/resolver/hub/resolver_test.go | 61 ++++++++++++++++--- 21 files changed, 423 insertions(+), 35 deletions(-) create mode 120000 cmd/resolvers/kodata/HEAD create mode 120000 cmd/resolvers/kodata/LICENSE create mode 120000 cmd/resolvers/kodata/refs create mode 120000 cmd/resolvers/kodata/third_party create mode 100644 cmd/resolvers/main.go create mode 100644 pkg/resolution/resolver/framework/testing/featureflag.go diff --git a/cmd/resolvers/kodata/HEAD b/cmd/resolvers/kodata/HEAD new file mode 120000 index 00000000000..8f63681d362 --- /dev/null +++ b/cmd/resolvers/kodata/HEAD @@ -0,0 +1 @@ +../../../.git/HEAD \ No newline at end of file diff --git a/cmd/resolvers/kodata/LICENSE b/cmd/resolvers/kodata/LICENSE new file mode 120000 index 00000000000..5853aaea53b --- /dev/null +++ b/cmd/resolvers/kodata/LICENSE @@ -0,0 +1 @@ +../../../LICENSE \ No newline at end of file diff --git a/cmd/resolvers/kodata/refs b/cmd/resolvers/kodata/refs new file mode 120000 index 00000000000..739d35bf96a --- /dev/null +++ b/cmd/resolvers/kodata/refs @@ -0,0 +1 @@ +../../../.git/refs \ No newline at end of file diff --git a/cmd/resolvers/kodata/third_party b/cmd/resolvers/kodata/third_party new file mode 120000 index 00000000000..6bfa480c09a --- /dev/null +++ b/cmd/resolvers/kodata/third_party @@ -0,0 +1 @@ +../../../third_party \ No newline at end of file diff --git a/cmd/resolvers/main.go b/cmd/resolvers/main.go new file mode 100644 index 00000000000..18de0c0f1a3 --- /dev/null +++ b/cmd/resolvers/main.go @@ -0,0 +1,37 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "github.com/tektoncd/pipeline/pkg/apis/resolution/v1alpha1" + "github.com/tektoncd/pipeline/pkg/resolution/resolver/bundle" + "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework" + "github.com/tektoncd/pipeline/pkg/resolution/resolver/git" + "github.com/tektoncd/pipeline/pkg/resolution/resolver/hub" + filteredinformerfactory "knative.dev/pkg/client/injection/kube/informers/factory/filtered" + "knative.dev/pkg/injection/sharedmain" + "knative.dev/pkg/signals" +) + +func main() { + ctx := filteredinformerfactory.WithSelectors(signals.NewContext(), v1alpha1.ManagedByLabelKey) + + sharedmain.MainWithContext(ctx, "controller", + framework.NewController(ctx, &git.Resolver{}), + framework.NewController(ctx, &hub.Resolver{}), + framework.NewController(ctx, &bundle.Resolver{})) +} diff --git a/docs/bundle-resolver.md b/docs/bundle-resolver.md index 0efec4d297e..b473d8ad20b 100644 --- a/docs/bundle-resolver.md +++ b/docs/bundle-resolver.md @@ -2,7 +2,7 @@ ## Resolver Type -This Resolver responds to type `bundle`. +This Resolver responds to type `bundles`. ## Parameters @@ -17,7 +17,7 @@ This Resolver responds to type `bundle`. - A cluster running Tekton Pipeline v0.40.0 or later, with the `alpha` feature gate enabled. - The [built-in remote resolvers installed](./install.md#installing-and-configuring-remote-task-and-pipeline-resolution). -- The `enable-tekton-oci-bundles` feature flag set to `true`. +- The `enable-bundles-resolver` feature flag set to `true`. ## Configuration diff --git a/docs/install.md b/docs/install.md index 68a642a1b81..d8898fd6118 100644 --- a/docs/install.md +++ b/docs/install.md @@ -287,7 +287,7 @@ Three remote resolvers are currently provided as part of the `resolvers.yaml` in By default, these remote resolvers are disabled. Each resolver is enabled by setting [the appropriate feature flag](#customizing-the-pipelines-controller-behavior). -1. [The `bundle` resolver](./bundle-resolver.md), enabled by setting the `enable-tekton-oci-bundles` +1. [The `bundles` resolver](./bundle-resolver.md), enabled by setting the `enable-bundles-resolver` feature flag to `true`. 1. [The `git` resolver](./git-resolver.md), enabled by setting the `enable-git-resolver` feature flag to `true`. diff --git a/docs/resolution-getting-started.md b/docs/resolution-getting-started.md index 4796767aca8..eaf8f6695df 100644 --- a/docs/resolution-getting-started.md +++ b/docs/resolution-getting-started.md @@ -37,7 +37,7 @@ kubectl patch -n tekton-pipelines configmap feature-flags -p '{"data":{"enable-a The feature flags for the built-in resolvers are: -* The `bundles` resolver: `enable-tekton-oci-bundles` +* The `bundles` resolver: `enable-bundles-resolver` * The `git` resolver: `enable-git-resolver` * The `hub` resolver: `enable-hub-resolver` diff --git a/docs/resolution.md b/docs/resolution.md index 0d807fd34b9..07d8851dc9b 100644 --- a/docs/resolution.md +++ b/docs/resolution.md @@ -7,7 +7,7 @@ For new users getting started with Tekton Pipeilne remote resolution, check out ## Built-in Resolvers -1. [The `bundle` resolver](./bundle-resolver.md), enabled by setting the `enable-tekton-oci-bundles` +1. [The `bundles` resolver](./bundle-resolver.md), enabled by setting the `enable-bundles-resolver` feature flag to `true`. 1. [The `git` resolver](./git-resolver.md), enabled by setting the `enable-git-resolver` feature flag to `true`. diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 555b28e5f3e..3011c6c6b57 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -60,6 +60,12 @@ const ( DefaultSendCloudEventsForRuns = false // DefaultEmbeddedStatus is the default value for "embedded-status". DefaultEmbeddedStatus = FullEmbeddedStatus + // DefaultEnableGitResolver is the default value for "enable-git-resolver". + DefaultEnableGitResolver = false + // DefaultEnableHubResolver is the default value for "enable-hub-resolver". + DefaultEnableHubResolver = false + // DefaultEnableBundlesResolver is the default value for "enable-bundles-resolver". + DefaultEnableBundlesResolver = false disableAffinityAssistantKey = "disable-affinity-assistant" disableCredsInitKey = "disable-creds-init" @@ -71,6 +77,13 @@ const ( enableAPIFields = "enable-api-fields" sendCloudEventsForRuns = "send-cloudevents-for-runs" embeddedStatus = "embedded-status" + + // EnableGitResolver is the flag used to enable the git remote resolver + EnableGitResolver = "enable-git-resolver" + // EnableHubResolver is the flag used to enable the hub remote resolver + EnableHubResolver = "enable-hub-resolver" + // EnableBundlesResolver is the flag used to enable the bundle remote resolver + EnableBundlesResolver = "enable-bundles-resolver" ) // FeatureFlags holds the features configurations @@ -87,6 +100,9 @@ type FeatureFlags struct { SendCloudEventsForRuns bool AwaitSidecarReadiness bool EmbeddedStatus string + EnableGitResolver bool + EnableHubResolver bool + EnableBundleResolver bool } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -138,6 +154,15 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setEmbeddedStatus(cfgMap, DefaultEmbeddedStatus, &tc.EmbeddedStatus); err != nil { return nil, err } + if err := setFeature(EnableGitResolver, DefaultEnableGitResolver, &tc.EnableGitResolver); err != nil { + return nil, err + } + if err := setFeature(EnableHubResolver, DefaultEnableHubResolver, &tc.EnableHubResolver); err != nil { + return nil, err + } + if err := setFeature(EnableBundlesResolver, DefaultEnableBundlesResolver, &tc.EnableBundleResolver); err != nil { + return nil, err + } // Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if // enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of @@ -147,6 +172,7 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { // defeat the purpose of having a single shared gate for all alpha features. if tc.EnableAPIFields == AlphaAPIFields { tc.EnableTektonOCIBundles = true + tc.EnableBundleResolver = true tc.EnableCustomTasks = true } else { if err := setFeature(enableTektonOCIBundles, DefaultEnableTektonOciBundles, &tc.EnableTektonOCIBundles); err != nil { diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 40104cc76a1..9d258a101ed 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -60,6 +60,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableAPIFields: "alpha", SendCloudEventsForRuns: true, EmbeddedStatus: "both", + EnableBundleResolver: true, }, fileName: "feature-flags-all-flags-set", }, @@ -78,6 +79,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts, SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns, EmbeddedStatus: config.DefaultEmbeddedStatus, + EnableBundleResolver: true, }, fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks", }, diff --git a/pkg/resolution/resolver/bundle/resolver.go b/pkg/resolution/resolver/bundle/resolver.go index f0c1ebe5597..a6ffc341357 100644 --- a/pkg/resolution/resolver/bundle/resolver.go +++ b/pkg/resolution/resolver/bundle/resolver.go @@ -18,15 +18,19 @@ package bundle import ( "context" + "errors" "time" "github.com/google/go-containerregistry/pkg/authn/k8schain" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/resolution/common" "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework" "k8s.io/client-go/kubernetes" "knative.dev/pkg/client/injection/kube/client" ) +const disabledError = "cannot handle resolution request, enable-bundles-resolver feature flag not true" + // LabelValueBundleResolverType is the value to use for the // resolution.tekton.dev/type label on resource requests const LabelValueBundleResolverType string = "bundles" @@ -65,6 +69,9 @@ func (r *Resolver) GetSelector(context.Context) map[string]string { // ValidateParams ensures parameters from a request are as expected. func (r *Resolver) ValidateParams(ctx context.Context, params map[string]string) error { + if r.isDisabled(ctx) { + return errors.New(disabledError) + } if _, err := OptionsFromParams(ctx, params); err != nil { return err } @@ -73,6 +80,9 @@ func (r *Resolver) ValidateParams(ctx context.Context, params map[string]string) // Resolve uses the given params to resolve the requested file or resource. func (r *Resolver) Resolve(ctx context.Context, params map[string]string) (framework.ResolvedResource, error) { + if r.isDisabled(ctx) { + return nil, errors.New(disabledError) + } opts, err := OptionsFromParams(ctx, params) if err != nil { return nil, err @@ -86,3 +96,12 @@ func (r *Resolver) Resolve(ctx context.Context, params map[string]string) (frame defer cancelFn() return GetEntry(ctx, kc, opts) } + +func (r *Resolver) isDisabled(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableTektonOCIBundles || cfg.FeatureFlags.EnableBundleResolver { + return false + } + + return true +} diff --git a/pkg/resolution/resolver/bundle/resolver_test.go b/pkg/resolution/resolver/bundle/resolver_test.go index 287a1f7653d..b79c4aa1a1a 100644 --- a/pkg/resolution/resolver/bundle/resolver_test.go +++ b/pkg/resolution/resolver/bundle/resolver_test.go @@ -20,7 +20,10 @@ import ( "context" "testing" + "github.com/google/go-cmp/cmp" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" + frtesting "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework/testing" + "github.com/tektoncd/pipeline/test/diff" ) func TestGetSelector(t *testing.T) { @@ -42,7 +45,7 @@ func TestValidateParams(t *testing.T) { ParamBundle: "bar", ParamServiceAccount: "baz", } - if err := resolver.ValidateParams(context.Background(), paramsWithTask); err != nil { + if err := resolver.ValidateParams(resolverContext(), paramsWithTask); err != nil { t.Fatalf("unexpected error validating params: %v", err) } @@ -52,11 +55,32 @@ func TestValidateParams(t *testing.T) { ParamBundle: "bar", ParamServiceAccount: "baz", } - if err := resolver.ValidateParams(context.Background(), paramsWithPipeline); err != nil { + if err := resolver.ValidateParams(resolverContext(), paramsWithPipeline); err != nil { t.Fatalf("unexpected error validating params: %v", err) } } +func TestValidateParamsDisabled(t *testing.T) { + resolver := Resolver{} + + var err error + + params := map[string]string{ + ParamKind: "task", + ParamName: "foo", + ParamBundle: "bar", + ParamServiceAccount: "baz", + } + err = resolver.ValidateParams(context.Background(), params) + if err == nil { + t.Fatalf("expected disabled err") + } + + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + func TestValidateParamsMissing(t *testing.T) { resolver := Resolver{} @@ -67,7 +91,7 @@ func TestValidateParamsMissing(t *testing.T) { ParamName: "foo", ParamServiceAccount: "baz", } - err = resolver.ValidateParams(context.Background(), paramsMissingBundle) + err = resolver.ValidateParams(resolverContext(), paramsMissingBundle) if err == nil { t.Fatalf("expected missing kind err") } @@ -77,9 +101,34 @@ func TestValidateParamsMissing(t *testing.T) { ParamBundle: "bar", ParamServiceAccount: "baz", } - err = resolver.ValidateParams(context.Background(), paramsMissingName) + err = resolver.ValidateParams(resolverContext(), paramsMissingName) if err == nil { t.Fatalf("expected missing name err") } } + +func TestResolveDisabled(t *testing.T) { + resolver := Resolver{} + + var err error + + params := map[string]string{ + ParamKind: "task", + ParamName: "foo", + ParamBundle: "bar", + ParamServiceAccount: "baz", + } + _, err = resolver.Resolve(context.Background(), params) + if err == nil { + t.Fatalf("expected disabled err") + } + + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + +func resolverContext() context.Context { + return frtesting.ContextWithBundlesResolverEnabled(context.Background()) +} diff --git a/pkg/resolution/resolver/framework/configstore.go b/pkg/resolution/resolver/framework/configstore.go index 8e200eba1d3..2ccb13d72fa 100644 --- a/pkg/resolution/resolver/framework/configstore.go +++ b/pkg/resolution/resolver/framework/configstore.go @@ -19,6 +19,7 @@ package framework import ( "context" + "github.com/tektoncd/pipeline/pkg/apis/config" corev1 "k8s.io/api/core/v1" "knative.dev/pkg/configmap" ) @@ -44,10 +45,34 @@ func DataFromConfigMap(config *corev1.ConfigMap) (map[string]string, error) { // ConfigStore wraps a knative untyped store and provides helper methods // for working with a resolver's configuration data. type ConfigStore struct { + *config.Store resolverConfigName string untyped *configmap.UntypedStore } +// NewConfigStore creates a new untyped store for the resolver's configuration and a config.Store for general Pipeline configuration. +func NewConfigStore(resolverConfigName string, logger configmap.Logger) *ConfigStore { + return &ConfigStore{ + Store: config.NewStore(logger), + resolverConfigName: resolverConfigName, + untyped: configmap.NewUntypedStore( + "resolver-config", + logger, + configmap.Constructors{ + resolverConfigName: DataFromConfigMap, + }, + ), + } +} + +// WatchConfigs uses the provided configmap.Watcher +// to setup watches for the config names provided in the +// Constructors map +func (store *ConfigStore) WatchConfigs(w configmap.Watcher) { + store.untyped.WatchConfigs(w) + store.Store.WatchConfigs(w) +} + // GetResolverConfig returns a copy of the resolver's current // configuration or an empty map if the stored config is nil or invalid. func (store *ConfigStore) GetResolverConfig() map[string]string { @@ -65,7 +90,7 @@ func (store *ConfigStore) GetResolverConfig() map[string]string { // data stored in it. func (store *ConfigStore) ToContext(ctx context.Context) context.Context { conf := store.GetResolverConfig() - return InjectResolverConfigToContext(ctx, conf) + return InjectResolverConfigToContext(store.Store.ToContext(ctx), conf) } // InjectResolverConfigToContext returns a new context with a diff --git a/pkg/resolution/resolver/framework/controller.go b/pkg/resolution/resolver/framework/controller.go index 5f2ffcdbb87..2145a46626d 100644 --- a/pkg/resolution/resolver/framework/controller.go +++ b/pkg/resolution/resolver/framework/controller.go @@ -167,17 +167,8 @@ func watchConfigChanges(ctx context.Context, reconciler *Reconciler, cmw configm if resolverConfigName == "" { panic("resolver returned empty config name") } - reconciler.configStore = &ConfigStore{ - resolverConfigName: resolverConfigName, - untyped: configmap.NewUntypedStore( - "resolver-config", - logger, - configmap.Constructors{ - resolverConfigName: DataFromConfigMap, - }, - ), - } - reconciler.configStore.untyped.WatchConfigs(cmw) + reconciler.configStore = NewConfigStore(resolverConfigName, logger) + reconciler.configStore.WatchConfigs(cmw) } } diff --git a/pkg/resolution/resolver/framework/testing/fakecontroller.go b/pkg/resolution/resolver/framework/testing/fakecontroller.go index 8846b648bda..eb1be824564 100644 --- a/pkg/resolution/resolver/framework/testing/fakecontroller.go +++ b/pkg/resolution/resolver/framework/testing/fakecontroller.go @@ -24,11 +24,13 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/resolution/v1alpha1" "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework" "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/clock" @@ -95,6 +97,7 @@ func GetResolverFrameworkController(ctx context.Context, t *testing.T, d test.Da func initializeResolverFrameworkControllerAssets(ctx context.Context, t *testing.T, d test.Data, resolver framework.Resolver, modifiers ...framework.ReconcilerModifier) (test.Assets, func()) { t.Helper() ctx, cancel := context.WithCancel(ctx) + ensureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) configMapWatcher := cminformer.NewInformedWatcher(c.Kube, system.Namespace()) ctl := framework.NewController(ctx, resolver, modifiers...)(ctx, configMapWatcher) @@ -125,3 +128,54 @@ func setClockOnReconciler(r *framework.Reconciler) { r.Clock = testClock } } + +func ensureConfigurationConfigMapsExist(d *test.Data) { + var defaultsExists, featureFlagsExists, artifactBucketExists, artifactPVCExists, metricsExists bool + for _, cm := range d.ConfigMaps { + if cm.Name == config.GetDefaultsConfigName() { + defaultsExists = true + } + if cm.Name == config.GetFeatureFlagsConfigName() { + featureFlagsExists = true + } + if cm.Name == config.GetArtifactBucketConfigName() { + artifactBucketExists = true + } + if cm.Name == config.GetArtifactPVCConfigName() { + artifactPVCExists = true + } + if cm.Name == config.GetMetricsConfigName() { + metricsExists = true + } + } + if !defaultsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{}, + }) + } + if !featureFlagsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{}, + }) + } + if !artifactBucketExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactBucketConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{}, + }) + } + if !artifactPVCExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetArtifactPVCConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{}, + }) + } + if !metricsExists { + d.ConfigMaps = append(d.ConfigMaps, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetMetricsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{}, + }) + } +} diff --git a/pkg/resolution/resolver/framework/testing/featureflag.go b/pkg/resolution/resolver/framework/testing/featureflag.go new file mode 100644 index 00000000000..bb085465064 --- /dev/null +++ b/pkg/resolution/resolver/framework/testing/featureflag.go @@ -0,0 +1,49 @@ +/* + Copyright 2022 The Tekton Authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +package testing + +import ( + "context" + + "github.com/tektoncd/pipeline/pkg/apis/config" +) + +// ContextWithGitResolverEnabled returns a context containing a Config with the enable-git-resolver feature flag enabled. +func ContextWithGitResolverEnabled(ctx context.Context) context.Context { + return contextWithResolverEnabled(ctx, "enable-git-resolver") +} + +// ContextWithHubResolverEnabled returns a context containing a Config with the enable-hub-resolver feature flag enabled. +func ContextWithHubResolverEnabled(ctx context.Context) context.Context { + return contextWithResolverEnabled(ctx, "enable-hub-resolver") +} + +// ContextWithBundlesResolverEnabled returns a context containing a Config with the enable-bundles-resolver feature flag enabled. +func ContextWithBundlesResolverEnabled(ctx context.Context) context.Context { + return contextWithResolverEnabled(ctx, "enable-bundles-resolver") +} + +func contextWithResolverEnabled(ctx context.Context, resolverFlag string) context.Context { + featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ + resolverFlag: "true", + }) + cfg := &config.Config{ + FeatureFlags: featureFlags, + } + return config.ToContext(ctx, cfg) +} diff --git a/pkg/resolution/resolver/git/resolver.go b/pkg/resolution/resolver/git/resolver.go index 989c28e9989..f4000cf8a33 100644 --- a/pkg/resolution/resolver/git/resolver.go +++ b/pkg/resolution/resolver/git/resolver.go @@ -30,10 +30,13 @@ import ( gitcfg "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/storage/memory" + "github.com/tektoncd/pipeline/pkg/apis/config" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework" ) +const disabledError = "cannot handle resolution request, enable-git-resolver feature flag not true" + // LabelValueGitResolverType is the value to use for the // resolution.tekton.dev/type label on resource requests const LabelValueGitResolverType string = "git" @@ -71,7 +74,11 @@ func (r *Resolver) GetSelector(_ context.Context) map[string]string { // ValidateParams returns an error if the given parameter map is not // valid for a resource request targeting the gitresolver. -func (r *Resolver) ValidateParams(_ context.Context, params map[string]string) error { +func (r *Resolver) ValidateParams(ctx context.Context, params map[string]string) error { + if r.isDisabled(ctx) { + return errors.New(disabledError) + } + required := []string{ PathParam, } @@ -99,6 +106,10 @@ func (r *Resolver) ValidateParams(_ context.Context, params map[string]string) e // Resolve performs the work of fetching a file from git given a map of // parameters. func (r *Resolver) Resolve(ctx context.Context, params map[string]string) (framework.ResolvedResource, error) { + if r.isDisabled(ctx) { + return nil, errors.New(disabledError) + } + conf := framework.GetResolverConfigFromContext(ctx) repo := params[URLParam] if repo == "" { @@ -197,6 +208,15 @@ func (r *Resolver) GetResolutionTimeout(ctx context.Context, defaultTimeout time return defaultTimeout } +func (r *Resolver) isDisabled(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableGitResolver { + return false + } + + return true +} + // ResolvedGitResource implements framework.ResolvedResource and returns // the resolved file []byte data and an annotation map for any metadata. type ResolvedGitResource struct { diff --git a/pkg/resolution/resolver/git/resolver_test.go b/pkg/resolution/resolver/git/resolver_test.go index 3c83214421b..f8bc5a73b2a 100644 --- a/pkg/resolution/resolver/git/resolver_test.go +++ b/pkg/resolution/resolver/git/resolver_test.go @@ -31,6 +31,7 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/resolution/v1alpha1" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" @@ -49,7 +50,7 @@ import ( func TestGetSelector(t *testing.T) { resolver := Resolver{} - sel := resolver.GetSelector(context.Background()) + sel := resolver.GetSelector(resolverContext()) if typ, has := sel[resolutioncommon.LabelKeyResolverType]; !has { t.Fatalf("unexpected selector: %v", sel) } else if typ != LabelValueGitResolverType { @@ -65,11 +66,29 @@ func TestValidateParams(t *testing.T) { RevisionParam: "baz", } - if err := resolver.ValidateParams(context.Background(), paramsWithRevision); err != nil { + if err := resolver.ValidateParams(resolverContext(), paramsWithRevision); err != nil { t.Fatalf("unexpected error validating params: %v", err) } } +func TestValidateParamsNotEnabled(t *testing.T) { + resolver := Resolver{} + + var err error + + someParams := map[string]string{ + PathParam: "bar", + RevisionParam: "baz", + } + err = resolver.ValidateParams(context.Background(), someParams) + if err == nil { + t.Fatalf("expected disabled err") + } + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + func TestValidateParamsMissing(t *testing.T) { resolver := Resolver{} @@ -79,7 +98,7 @@ func TestValidateParamsMissing(t *testing.T) { URLParam: "foo", RevisionParam: "baz", } - err = resolver.ValidateParams(context.Background(), paramsMissingPath) + err = resolver.ValidateParams(resolverContext(), paramsMissingPath) if err == nil { t.Fatalf("expected missing pathInRepo err") } @@ -88,7 +107,7 @@ func TestValidateParamsMissing(t *testing.T) { func TestGetResolutionTimeoutDefault(t *testing.T) { resolver := Resolver{} defaultTimeout := 30 * time.Minute - timeout := resolver.GetResolutionTimeout(context.Background(), defaultTimeout) + timeout := resolver.GetResolutionTimeout(resolverContext(), defaultTimeout) if timeout != defaultTimeout { t.Fatalf("expected default timeout to be returned") } @@ -101,13 +120,31 @@ func TestGetResolutionTimeoutCustom(t *testing.T) { config := map[string]string{ ConfigFieldTimeout: configTimeout.String(), } - ctx := framework.InjectResolverConfigToContext(context.Background(), config) + ctx := framework.InjectResolverConfigToContext(resolverContext(), config) timeout := resolver.GetResolutionTimeout(ctx, defaultTimeout) if timeout != configTimeout { t.Fatalf("expected timeout from config to be returned") } } +func TestResolveNotEnabled(t *testing.T) { + resolver := Resolver{} + + var err error + + someParams := map[string]string{ + PathParam: "bar", + RevisionParam: "baz", + } + _, err = resolver.Resolve(context.Background(), someParams) + if err == nil { + t.Fatalf("expected disabled err") + } + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + func TestResolve(t *testing.T) { withTemporaryGitConfig(t) @@ -228,7 +265,7 @@ func TestResolve(t *testing.T) { v := map[string]string{ ConfigRevision: plumbing.Master.Short(), } - output, err := resolver.Resolve(context.WithValue(context.Background(), struct{}{}, v), params) + output, err := resolver.Resolve(context.WithValue(resolverContext(), struct{}{}, v), params) if tc.expectedErr != nil { if err == nil { t.Fatalf("expected err '%v' but didn't get one", tc.expectedErr) @@ -457,6 +494,11 @@ func TestController(t *testing.T) { ConfigFieldTimeout: "1m", ConfigRevision: plumbing.Master.Short(), }, + }, { + ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-git-resolver": "true", + }, }}, ResolutionRequests: []*v1alpha1.ResolutionRequest{request}, } @@ -641,3 +683,7 @@ func createRequest(repoURL, pathInRepo, revision, specificCommit string, useNthC return rr } + +func resolverContext() context.Context { + return frtesting.ContextWithGitResolverEnabled(context.Background()) +} diff --git a/pkg/resolution/resolver/hub/resolver.go b/pkg/resolution/resolver/hub/resolver.go index 302ec95441b..1d5f30eb4bd 100644 --- a/pkg/resolution/resolver/hub/resolver.go +++ b/pkg/resolution/resolver/hub/resolver.go @@ -21,10 +21,13 @@ import ( "io" "net/http" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/resolution/common" "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework" ) +const disabledError = "cannot handle resolution request, enable-hub-resolver feature flag not true" + // LabelValueHubResolverType is the value to use for the // resolution.tekton.dev/type label on resource requests const LabelValueHubResolverType string = "hub" @@ -59,6 +62,9 @@ func (r *Resolver) GetSelector(context.Context) map[string]string { // ValidateParams ensures parameters from a request are as expected. func (r *Resolver) ValidateParams(ctx context.Context, params map[string]string) error { + if r.isDisabled(ctx) { + return errors.New(disabledError) + } if _, ok := params[ParamName]; !ok { return errors.New("must include name param") } @@ -83,6 +89,10 @@ type hubResponse struct { // Resolve uses the given params to resolve the requested file or resource. func (r *Resolver) Resolve(ctx context.Context, params map[string]string) (framework.ResolvedResource, error) { + if r.isDisabled(ctx) { + return nil, errors.New(disabledError) + } + conf := framework.GetResolverConfigFromContext(ctx) if _, ok := params[ParamCatalog]; !ok { if catalogString, ok := conf[ConfigCatalog]; ok { @@ -142,3 +152,12 @@ func (rr *ResolvedHubResource) Data() []byte { func (*ResolvedHubResource) Annotations() map[string]string { return nil } + +func (r *Resolver) isDisabled(ctx context.Context) bool { + cfg := config.FromContextOrDefaults(ctx) + if cfg.FeatureFlags.EnableHubResolver { + return false + } + + return true +} diff --git a/pkg/resolution/resolver/hub/resolver_test.go b/pkg/resolution/resolver/hub/resolver_test.go index ea38e16de11..31bdda3d7df 100644 --- a/pkg/resolution/resolver/hub/resolver_test.go +++ b/pkg/resolution/resolver/hub/resolver_test.go @@ -25,12 +25,13 @@ import ( "github.com/google/go-cmp/cmp" resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common" + frtesting "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework/testing" "github.com/tektoncd/pipeline/test/diff" ) func TestGetSelector(t *testing.T) { resolver := Resolver{} - sel := resolver.GetSelector(context.Background()) + sel := resolver.GetSelector(resolverContext()) if typ, has := sel[resolutioncommon.LabelKeyResolverType]; !has { t.Fatalf("unexpected selector: %v", sel) } else if typ != LabelValueHubResolverType { @@ -47,7 +48,7 @@ func TestValidateParams(t *testing.T) { ParamVersion: "bar", ParamCatalog: "baz", } - if err := resolver.ValidateParams(context.Background(), paramsWithTask); err != nil { + if err := resolver.ValidateParams(resolverContext(), paramsWithTask); err != nil { t.Fatalf("unexpected error validating params: %v", err) } @@ -57,11 +58,32 @@ func TestValidateParams(t *testing.T) { ParamVersion: "bar", ParamCatalog: "baz", } - if err := resolver.ValidateParams(context.Background(), paramsWithPipeline); err != nil { + if err := resolver.ValidateParams(resolverContext(), paramsWithPipeline); err != nil { t.Fatalf("unexpected error validating params: %v", err) } } +func TestValidateParamsDisabled(t *testing.T) { + resolver := Resolver{} + + var err error + + params := map[string]string{ + ParamKind: "task", + ParamName: "foo", + ParamVersion: "bar", + ParamCatalog: "baz", + } + err = resolver.ValidateParams(context.Background(), params) + if err == nil { + t.Fatalf("expected missing name err") + } + + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + func TestValidateParamsMissing(t *testing.T) { resolver := Resolver{} @@ -71,7 +93,7 @@ func TestValidateParamsMissing(t *testing.T) { ParamKind: "foo", ParamVersion: "bar", } - err = resolver.ValidateParams(context.Background(), paramsMissingName) + err = resolver.ValidateParams(resolverContext(), paramsMissingName) if err == nil { t.Fatalf("expected missing name err") } @@ -80,7 +102,7 @@ func TestValidateParamsMissing(t *testing.T) { ParamKind: "foo", ParamName: "bar", } - err = resolver.ValidateParams(context.Background(), paramsMissingVersion) + err = resolver.ValidateParams(resolverContext(), paramsMissingVersion) if err == nil { t.Fatalf("expected missing version err") } @@ -94,12 +116,33 @@ func TestValidateParamsConflictingKindName(t *testing.T) { ParamVersion: "bar", ParamCatalog: "baz", } - err := resolver.ValidateParams(context.Background(), params) + err := resolver.ValidateParams(resolverContext(), params) if err == nil { t.Fatalf("expected err due to conflicting kind param") } } +func TestResolveDisabled(t *testing.T) { + resolver := Resolver{} + + var err error + + params := map[string]string{ + ParamKind: "task", + ParamName: "foo", + ParamVersion: "bar", + ParamCatalog: "baz", + } + _, err = resolver.Resolve(context.Background(), params) + if err == nil { + t.Fatalf("expected missing name err") + } + + if d := cmp.Diff(disabledError, err.Error()); d != "" { + t.Errorf("unexpected error: %s", diff.PrintWantGot(d)) + } +} + func TestResolve(t *testing.T) { testCases := []struct { name string @@ -163,7 +206,7 @@ func TestResolve(t *testing.T) { ParamCatalog: tc.catalog, } - output, err := resolver.Resolve(context.Background(), params) + output, err := resolver.Resolve(resolverContext(), params) if tc.expectedErr != nil { if err == nil { t.Fatalf("expected err '%v' but didn't get one", tc.expectedErr) @@ -187,3 +230,7 @@ func TestResolve(t *testing.T) { }) } } + +func resolverContext() context.Context { + return frtesting.ContextWithHubResolverEnabled(context.Background()) +}