Disable upload service usage if documentation enabled flag is set to …

…false

charts/helm-broker/templates/deploy.yaml

@@ -148,8 +148,8 @@ spec:
         configMap:
           name: helm-config-map
       - name: ssh-cfg
-          configMap:
-            name: ssh-cfg
+        configMap:
+          name: ssh-cfg
       - name: helm-certs
         secret:
           secretName: helm-secret

internal/addon/provider/client.go

@@ -20,19 +20,22 @@ type Client struct {
 	log                 *logrus.Entry
 	addonLoader         addonLoader
 	concreteGetter      RepositoryGetter
+
+	documentationEnabled bool
 }
 
 // NewClient returns new instance of Client
-func NewClient(concreteGetter RepositoryGetter, addonLoader addonLoader, log logrus.FieldLogger) (*Client, error) {
+func NewClient(concreteGetter RepositoryGetter, addonLoader addonLoader, documentationEnabled bool, log logrus.FieldLogger) (*Client, error) {
 	specifiedSchemRegex, err := regexp.Compile(`^([A-Za-z0-9]+)::(.+)$`)
 	if err != nil {
 		return nil, err
 	}
 	return &Client{
-		specifiedSchemRegex: specifiedSchemRegex,
-		addonLoader:         addonLoader,
-		log:                 log.WithField("service", "addonClient"),
-		concreteGetter:      concreteGetter,
+		specifiedSchemRegex:  specifiedSchemRegex,
+		addonLoader:          addonLoader,
+		log:                  log.WithField("service", "addonClient"),
+		concreteGetter:       concreteGetter,
+		documentationEnabled: documentationEnabled,
 	}, nil
 }
 
@@ -47,9 +50,12 @@ func (d *Client) GetCompleteAddon(entry internal.IndexEntry) (internal.AddonWith
 	if err != nil {
 		return internal.AddonWithCharts{}, errors.Wrapf(err, "while loading addon %v", entry.Name)
 	}
-	a.RepositoryURL, err = d.concreteGetter.AddonDocURL(entry.Name, entry.Version)
-	if err != nil {
-		return internal.AddonWithCharts{}, errors.Wrapf(err, "while getting Docs URL for addon %v", entry.Name)
+
+	if d.documentationEnabled {
+		a.RepositoryURL, err = d.concreteGetter.AddonDocURL(entry.Name, entry.Version)
+		if err != nil {
+			return internal.AddonWithCharts{}, errors.Wrapf(err, "while getting Docs URL for addon %v", entry.Name)
+		}
 	}
 
 	return internal.AddonWithCharts{

internal/addon/provider/client_test.go

@@ -24,7 +24,7 @@ func TestRepositoryClientSuccess(t *testing.T) {
 	require.NoError(t, err)
 	defer os.RemoveAll(tmpDir)
 
-	addonLoader, err := provider.NewClient(fakeRepo, addon.NewLoader(tmpDir, log), log)
+	addonLoader, err := provider.NewClient(fakeRepo, addon.NewLoader(tmpDir, log), true, log)
 	require.NoError(t, err)
 
 	entry := internal.IndexEntry{
@@ -42,6 +42,7 @@ func TestRepositoryClientSuccess(t *testing.T) {
 
 	require.NoError(t, gotAddonErr)
 	assert.NotEmpty(t, gotAddon)
+	assert.NotEmpty(t, gotAddon.Addon.RepositoryURL)
 }
 
 // fakeRepository provide access to addons repository

internal/addon/provider/factory.go

@@ -23,23 +23,25 @@ type addonLoader interface {
 
 // ClientFactory knows how to build the concrete RepositoryGetter for given addon repository URL.
 type ClientFactory struct {
-	gettersProviders    map[string]Provider
-	specifiedSchemRegex *regexp.Regexp
-	log                 *logrus.Entry
-	addonLoader         addonLoader
+	gettersProviders            map[string]Provider
+	specifiedSchemRegex         *regexp.Regexp
+	log                         *logrus.Entry
+	addonLoader                 addonLoader
+	documentationEnabledEnabled bool
 }
 
 // NewClientFactory returns new instance of the ClientFactory
-func NewClientFactory(allowedGetters map[string]Provider, addonLoader addonLoader, log logrus.FieldLogger) (*ClientFactory, error) {
+func NewClientFactory(allowedGetters map[string]Provider, addonLoader addonLoader, documentationEnabled bool, log logrus.FieldLogger) (*ClientFactory, error) {
 	specifiedSchemRegex, err := regexp.Compile(`^([A-Za-z0-9]+)::(.+)$`)
 	if err != nil {
 		return nil, err
 	}
 	return &ClientFactory{
-		specifiedSchemRegex: specifiedSchemRegex,
-		gettersProviders:    allowedGetters,
-		addonLoader:         addonLoader,
-		log:                 log.WithField("service", "addonClientFactory"),
+		specifiedSchemRegex:         specifiedSchemRegex,
+		gettersProviders:            allowedGetters,
+		addonLoader:                 addonLoader,
+		log:                         log.WithField("service", "addonClientFactory"),
+		documentationEnabledEnabled: documentationEnabled,
 	}, nil
 }
 
@@ -70,7 +72,7 @@ func (cli *ClientFactory) NewGetter(rawURL, instPath string) (AddonClient, error
 		return nil, err
 	}
 
-	addonClient, err := NewClient(concreteGetter, cli.addonLoader, cli.log.WithField("getterScheme", scheme))
+	addonClient, err := NewClient(concreteGetter, cli.addonLoader, cli.documentationEnabledEnabled, cli.log.WithField("getterScheme", scheme))
 	if err != nil {
 		return nil, err
 	}

internal/addon/provider/git.go

@@ -109,13 +109,15 @@ func (g *GitGetter) AddonDocURL(name internal.AddonName, version internal.AddonV
 	if err != nil {
 		return "", exerr.Wrapf(err, "while creating archive '%s'", pathToTgz)
 	}
+	defer func() {
+		os.Remove(pathToTgz)
+	}()
 
 	file, err := os.Open(pathToTgz)
 	if err != nil {
 		return "", exerr.Wrapf(err, "while opening file '%s'", pathToTgz)
 	}
 	defer func() {
-		os.Remove(pathToTgz)
 		file.Close()
 	}()
 

internal/controller/setup.go

@@ -72,7 +72,7 @@ func SetupAndStartController(cfg *rest.Config, ctrCfg *config.ControllerConfig,
 		lg.Infof("Disabling support for HTTP protocol because DevelopMode is set to false.")
 	}
 
-	addonGetterFactory, err := provider.NewClientFactory(allowedGetters, addon.NewLoader(ctrCfg.TmpDir, lg), lg)
+	addonGetterFactory, err := provider.NewClientFactory(allowedGetters, addon.NewLoader(ctrCfg.TmpDir, lg), ctrCfg.DocumentationEnabled, lg)
 	fatalOnError(err, "cannot setup addon getter")
 
 	// Creating controllers

test/integration/integration_test.go

@@ -341,22 +341,61 @@ func TestDisabledDocs(t *testing.T) {
 	defer suite.tearDown()
 
 	t.Run("namespaced", func(t *testing.T) {
+		// given
 		suite.assertNoServicesInCatalogEndpoint("ns/stage")
 
 		// when
-		suite.createAddonsConfiguration("stage", "addon1", []string{accTestRepo}, sourceHTTP)
+		suite.createAddonsConfiguration("stage", "addon1", []string{redisRepo}, sourceHTTP)
 
 		// then
 		suite.waitForAddonsConfigurationPhase("stage", "addon1", v1alpha1.AddonsConfigurationReady)
+		suite.waitForServicesInCatalogEndpoint("ns/stage", []string{redisAddonID})
+
+		suite.deleteAddonsConfiguration("stage", "addon1")
+		suite.waitForEmptyCatalogResponse("ns/stage")
+	})
+	t.Run("namespaced-git", func(t *testing.T) {
+		// given
+		suite.assertNoServicesInCatalogEndpoint("ns/stage")
+
+		// when
+		suite.createAddonsConfiguration("stage", "addon2", []string{redisRepo}, sourceGit)
+
+		// then
+		suite.waitForAddonsConfigurationPhase("stage", "addon2", v1alpha1.AddonsConfigurationReady)
+		suite.waitForServicesInCatalogEndpoint("ns/stage", []string{redisAddonIDGit})
+
+		suite.deleteAddonsConfiguration("stage", "addon2")
+		suite.waitForEmptyCatalogResponse("ns/stage")
 	})
 
 	t.Run("cluster", func(t *testing.T) {
+		// given
 		suite.assertNoServicesInCatalogEndpoint("cluster")
 
 		// when
-		suite.createClusterAddonsConfiguration("addon1", []string{accTestRepo}, sourceHTTP)
+		suite.createClusterAddonsConfiguration("addon1", []string{redisRepo}, sourceHTTP)
 
 		// then
 		suite.waitForClusterAddonsConfigurationPhase("addon1", v1alpha1.AddonsConfigurationReady)
+		suite.waitForServicesInCatalogEndpoint("cluster", []string{redisAddonID})
+
+		suite.deleteClusterAddonsConfiguration("addon1")
+		suite.waitForEmptyCatalogResponse("cluster")
+	})
+
+	t.Run("cluster-Git", func(t *testing.T) {
+		// given
+		suite.assertNoServicesInCatalogEndpoint("cluster")
+
+		// when
+		suite.createClusterAddonsConfiguration("addon2", []string{redisRepo}, sourceGit)
+
+		// then
+		suite.waitForClusterAddonsConfigurationPhase("addon2", v1alpha1.AddonsConfigurationReady)
+		suite.waitForServicesInCatalogEndpoint("cluster", []string{redisAddonIDGit})
+
+		suite.deleteClusterAddonsConfiguration("addon2")
+		suite.waitForEmptyCatalogResponse("cluster")
 	})
 }

test/integration/suite_test.go

@@ -4,6 +4,7 @@ package integration_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -18,7 +19,7 @@ import (
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -122,7 +123,11 @@ func newTestSuite(t *testing.T, docsEnabled, httpBasicAuth bool) *testSuite {
 	}
 
 	uploadClient := &automock.Client{}
-	uploadClient.On("Upload", mock.AnythingOfType("string"), mock.Anything).Return(assetstore.UploadedFile{}, nil)
+	if docsEnabled {
+		uploadClient.On("Upload", mock.AnythingOfType("string"), mock.Anything).Return(assetstore.UploadedFile{}, nil)
+	} else {
+		uploadClient.On("Upload", mock.AnythingOfType("string"), mock.Anything).Return(assetstore.UploadedFile{}, errors.New("Upload must not be called, the service does not exists"))
+	}
 
 	mgr := controller.SetupAndStartController(restConfig, &config.ControllerConfig{
 		DevelopMode:              true, // DevelopMode allows "http" urls
@@ -414,7 +419,7 @@ func (ts *testSuite) assertDocsTopicExist(namespace, name string) {
 	err := wait.Poll(1*time.Second, 30*time.Second, func() (done bool, err error) {
 		key := types.NamespacedName{Name: name, Namespace: namespace}
 		err = ts.dynamicClient.Get(context.TODO(), key, &docsTopic)
-		if errors.IsNotFound(err) {
+		if apierrors.IsNotFound(err) {
 			ts.t.Logf("DocsTopic %q not found. Retry...", key)
 			return false, nil
 		}
@@ -434,7 +439,7 @@ func (ts *testSuite) assertClusterDocsTopicExist(name string) {
 	err := wait.Poll(1*time.Second, 30*time.Second, func() (done bool, err error) {
 		key := types.NamespacedName{Name: name}
 		err = ts.dynamicClient.Get(context.TODO(), key, &clusterDocsTopic)
-		if errors.IsNotFound(err) {
+		if apierrors.IsNotFound(err) {
 			ts.t.Logf("ClusterDocsTopic %q not found. Retry...", key)
 			return false, nil
 		}