Fix after review

SAP-archive · Aug 26, 2019 · 38ba5c9 · 38ba5c9
1 parent 0a441b7
commit 38ba5c9
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 25 deletions.
diff --git a/internal/controller/addons_controller_test.go b/internal/controller/addons_controller_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
 	"github.com/kyma-project/helm-broker/internal"
 	"github.com/kyma-project/helm-broker/internal/controller/automock"
+	"github.com/kyma-project/helm-broker/internal/controller/repository"
 	"github.com/kyma-project/helm-broker/internal/platform/logger/spy"
 	"github.com/kyma-project/helm-broker/internal/storage"
 	"github.com/kyma-project/helm-broker/pkg/apis"
@@ -416,7 +417,7 @@ type testSuite struct {
 	brokerFacade       *automock.BrokerFacade
 	docsProvider       *automock.DocsProvider
 	brokerSyncer       *automock.BrokerSyncer
-	templateService    *automock.TemplateService
+	templateService    *repository.Template
 	addonStorage       storage.Addon
 	chartStorage       storage.Chart
 }
@@ -431,15 +432,16 @@ func getTestSuite(t *testing.T, objects ...runtime.Object) *testSuite {
 	sFact, err := storage.NewFactory(storage.NewConfigListAllMemory())
 	require.NoError(t, err)
 
+	cli := fake.NewFakeClientWithScheme(sch, objects...)
 	ts := &testSuite{
 		t:                  t,
-		mgr:                getFakeManager(t, fake.NewFakeClientWithScheme(sch, objects...), sch),
+		mgr:                getFakeManager(t, cli, sch),
 		brokerFacade:       &automock.BrokerFacade{},
 		addonGetterFactory: &automock.AddonGetterFactory{},
 		addonGetter:        &automock.AddonGetter{},
 		brokerSyncer:       &automock.BrokerSyncer{},
 		docsProvider:       &automock.DocsProvider{},
-		templateService:    &automock.TemplateService{},
+		templateService:    repository.NewTemplate(cli),
 
 		addonStorage: sFact.Addon(),
 		chartStorage: sFact.Chart(),
@@ -448,7 +450,6 @@ func getTestSuite(t *testing.T, objects ...runtime.Object) *testSuite {
 	ts.brokerFacade.On("SetNamespace", fixAddonsConfiguration().Namespace).Return(nil).Once()
 	ts.brokerSyncer.On("SetNamespace", fixAddonsConfiguration().Namespace).Return(nil).Once()
 	ts.docsProvider.On("SetNamespace", fixAddonsConfiguration().Namespace).Return(nil).Once()
-	ts.templateService.On("SetNamespace", fixAddonsConfiguration().Namespace).Return(nil).Once()
 
 	return ts
 }
@@ -465,7 +466,6 @@ func (ts *testSuite) assertExpectations() {
 	ts.docsProvider.AssertExpectations(ts.t)
 	ts.brokerSyncer.AssertExpectations(ts.t)
 	ts.addonGetterFactory.AssertExpectations(ts.t)
-	ts.templateService.AssertExpectations(ts.t)
 }
 
 func (fakeManager) Add(manager.Runnable) error {

diff --git a/internal/controller/cluster_addons_controller_test.go b/internal/controller/cluster_addons_controller_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
 	"github.com/kyma-project/helm-broker/internal/controller/automock"
+	"github.com/kyma-project/helm-broker/internal/controller/repository"
 	"github.com/kyma-project/helm-broker/internal/storage"
 	"github.com/kyma-project/helm-broker/pkg/apis"
 	"github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1"
@@ -255,7 +256,7 @@ type clusterTestSuite struct {
 	brokerFacade       *automock.BrokerFacade
 	docsProvider       *automock.DocsProvider
 	brokerSyncer       *automock.BrokerSyncer
-	templateService    *automock.TemplateService
+	templateService    *repository.Template
 	addonStorage       storage.Addon
 	chartStorage       storage.Chart
 }
@@ -270,15 +271,16 @@ func getClusterTestSuite(t *testing.T, objects ...runtime.Object) *clusterTestSu
 	sFact, err := storage.NewFactory(storage.NewConfigListAllMemory())
 	require.NoError(t, err)
 
+	cli := fake.NewFakeClientWithScheme(sch, objects...)
 	return &clusterTestSuite{
 		t:                  t,
-		mgr:                getFakeManager(t, fake.NewFakeClientWithScheme(sch, objects...), sch),
+		mgr:                getFakeManager(t, cli, sch),
 		brokerFacade:       &automock.BrokerFacade{},
 		addonGetterFactory: &automock.AddonGetterFactory{},
 		addonGetter:        &automock.AddonGetter{},
 		brokerSyncer:       &automock.BrokerSyncer{},
 		docsProvider:       &automock.DocsProvider{},
-		templateService:    &automock.TemplateService{},
+		templateService:    repository.NewTemplate(cli),
 
 		addonStorage: sFact.Addon(),
 		chartStorage: sFact.Chart(),
@@ -291,7 +293,6 @@ func (ts *clusterTestSuite) assertExpectations() {
 	ts.addonGetter.AssertExpectations(ts.t)
 	ts.brokerSyncer.AssertExpectations(ts.t)
 	ts.addonGetterFactory.AssertExpectations(ts.t)
-	ts.templateService.AssertExpectations(ts.t)
 }
 
 func fixClusterAddonsConfiguration() *v1alpha1.ClusterAddonsConfiguration {

diff --git a/internal/controller/common.go b/internal/controller/common.go
@@ -250,17 +250,12 @@ func (c *common) loadRepositories(repos []v1alpha1.SpecRepository) *repository.C
 		c.log.Infof("- create addons for %q repository", specRepository.URL)
 		repo := repository.NewAddonsRepository(specRepository.URL)
 
-		addonsURL := specRepository.URL
-		if specRepository.SecretRef != nil {
-			c.log.Infof("- templating URL using secret `%s/%s`", specRepository.SecretRef.Name, specRepository.SecretRef.Namespace)
-			templateURL, err := c.templateService.TemplateURL(specRepository)
-			if err != nil {
-				repo.TemplatingError(err)
-				repositories.AddRepository(repo)
-				c.log.Errorf("while templating repository URL `%s`: %v", specRepository.URL, err)
-				continue
-			}
-			addonsURL = templateURL
+		addonsURL, err := c.templateService.TemplateURL(specRepository)
+		if err != nil {
+			repo.TemplatingError(err)
+			repositories.AddRepository(repo)
+			c.log.Errorf("while templating repository URL `%s`: %v", specRepository.URL, err)
+			continue
 		}
 
 		adds, err := c.createAddons(addonsURL)

diff --git a/internal/controller/repository/template.go b/internal/controller/repository/template.go
@@ -7,6 +7,8 @@ import (
 
 	"regexp"
 
+	"net/url"
+
 	"github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1"
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
@@ -21,6 +23,8 @@ type Template struct {
 	namespace string
 }
 
+var reg = regexp.MustCompile("\\{(.*?)\\}")
+
 // NewTemplate returns a new Template service
 func NewTemplate(cli client.Client) *Template {
 	return &Template{
@@ -39,6 +43,9 @@ func (t *Template) TemplateURL(repository v1alpha1.SpecRepository) (string, erro
 	if len(templateParameters) == 0 {
 		return repository.URL, nil
 	}
+	if repository.SecretRef == nil {
+		return "", fmt.Errorf("template fields `%v` provided but secretRef is empty", templateParameters)
+	}
 
 	fetchNS := t.namespace
 	if repository.SecretRef.Namespace != "" {
@@ -58,14 +65,12 @@ func (t *Template) TemplateURL(repository v1alpha1.SpecRepository) (string, erro
 		if !ok {
 			return "", fmt.Errorf("secret does not contain `%s` field", fieldName)
 		}
-		result = strings.Replace(result, val, string(tmp), -1)
+		result = strings.Replace(result, val, url.QueryEscape(string(tmp)), -1)
 	}
 
 	return strings.Replace(result, "\n", "", -1), nil
 }
 
 func (t *Template) findURLTemplates(url string) []string {
-	reg := regexp.MustCompile("\\{(.*?)\\}")
-
 	return reg.FindAllString(url, -1)
 }
diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go
@@ -62,7 +62,7 @@ const (
 	sourceHTTP = "http"
 	sourceGit  = "git"
 
-	basicPassword = "pAssword"
+	basicPassword = "pAssword{"
 	basicUsername = "user001"
 )
 
@@ -379,7 +379,6 @@ func WithSecretReference(namespace, name string) func(r *v1alpha1.SpecRepository
 func WithHTTPBasicAuth(username, password string) func(r *v1alpha1.SpecRepository) {
 	return func(r *v1alpha1.SpecRepository) {
 		r.URL = strings.Replace(r.URL, "http://", fmt.Sprintf("http://%s:%s@", username, password), 1)
-		fmt.Println(r.URL)
 	}
 }