diff --git a/internal/addon/provider/client.go b/internal/addon/provider/client.go index e7dfd18f..a660a07a 100644 --- a/internal/addon/provider/client.go +++ b/internal/addon/provider/client.go @@ -43,16 +43,17 @@ func (d *Client) Cleanup() error { // GetCompleteAddon returns a addon with his charts as AddonWithCharts instance. func (d *Client) GetCompleteAddon(entry internal.IndexEntry) (internal.AddonWithCharts, error) { - b, c, err := d.loadAddonAndCharts(entry.Name, entry.Version) + a, c, err := d.loadAddonAndCharts(entry.Name, entry.Version) if err != nil { return internal.AddonWithCharts{}, errors.Wrapf(err, "while loading addon %v", entry.Name) } - b.RepositoryURL, err = d.concreteGetter.AddonDocURL(entry.Name, entry.Version) + 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{ - Addon: b, + Addon: a, Charts: c, }, nil } diff --git a/internal/addon/provider/factory.go b/internal/addon/provider/factory.go index 36fa320d..da190f38 100644 --- a/internal/addon/provider/factory.go +++ b/internal/addon/provider/factory.go @@ -51,10 +51,6 @@ func (cli *ClientFactory) NewGetter(rawURL, instPath string) (AddonClient, error return nil, err } - if fullRealAddr != rawURL { - cli.log.Infof("[TRACE] go-getter detectors rewrote %q to %q", rawURL, fullRealAddr) - } - // get schema + source address fullRealAddrURL, err := url.Parse(fullRealAddr) if err != nil { diff --git a/internal/controller/addons_controller.go b/internal/controller/addons_controller.go index 5304d9a3..f596265f 100644 --- a/internal/controller/addons_controller.go +++ b/internal/controller/addons_controller.go @@ -27,12 +27,12 @@ type ReconcileAddonsConfiguration struct { } // NewReconcileAddonsConfiguration returns a new reconcile.Reconciler -func NewReconcileAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { +func NewReconcileAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { return &ReconcileAddonsConfiguration{ log: log.WithField("controller", "addons"), Client: mgr.GetClient(), - common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, docsProvider, brokerSyncer, brokerFacade, path.Join(tmpDir, "addon-loader-dst"), log), + common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, docsProvider, brokerSyncer, brokerFacade, templateService, path.Join(tmpDir, "addon-loader-dst"), log), } } diff --git a/internal/controller/addons_controller_test.go b/internal/controller/addons_controller_test.go index 61d6165f..cc340ed6 100644 --- 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" @@ -60,7 +61,7 @@ func TestReconcileAddonsConfiguration_AddAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -96,7 +97,7 @@ func TestReconcileAddonsConfiguration_AddAddonsProcess_ErrorIfBrokerExist(t *tes defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -135,7 +136,7 @@ func TestReconcileAddonsConfiguration_UpdateAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -166,7 +167,7 @@ func TestReconcileAddonsConfiguration_UpdateAddonsProcess_ConflictingAddons(t *t defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -190,7 +191,7 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -214,7 +215,7 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherAddons(t defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -242,7 +243,7 @@ func TestReconcileAddonsConfiguration_DeleteAddonsProcess_Error(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, tmpDir, spy.NewLogDummy()) + reconciler := NewReconcileAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, tmpDir, spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Namespace: fixAddonsCfg.Namespace, Name: fixAddonsCfg.Name}}) @@ -416,6 +417,7 @@ type testSuite struct { brokerFacade *automock.BrokerFacade docsProvider *automock.DocsProvider brokerSyncer *automock.BrokerSyncer + templateService *repository.Template addonStorage storage.Addon chartStorage storage.Chart } @@ -430,14 +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: repository.NewTemplate(cli), addonStorage: sFact.Addon(), chartStorage: sFact.Chart(), diff --git a/internal/controller/automock/template_service.go b/internal/controller/automock/template_service.go new file mode 100644 index 00000000..da9d53fb --- /dev/null +++ b/internal/controller/automock/template_service.go @@ -0,0 +1,36 @@ +// Code generated by mockery v1.0.0 +package automock + +import mock "github.com/stretchr/testify/mock" +import v1alpha1 "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + +// TemplateService is an autogenerated mock type for the TemplateService type +type TemplateService struct { + mock.Mock +} + +// SetNamespace provides a mock function with given fields: namespace +func (_m *TemplateService) SetNamespace(namespace string) { + _m.Called(namespace) +} + +// TemplateURL provides a mock function with given fields: repository +func (_m *TemplateService) TemplateURL(repository v1alpha1.SpecRepository) (string, error) { + ret := _m.Called(repository) + + var r0 string + if rf, ok := ret.Get(0).(func(v1alpha1.SpecRepository) string); ok { + r0 = rf(repository) + } else { + r0 = ret.Get(0).(string) + } + + var r1 error + if rf, ok := ret.Get(1).(func(v1alpha1.SpecRepository) error); ok { + r1 = rf(repository) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/internal/controller/cluster_addons_controller.go b/internal/controller/cluster_addons_controller.go index 96c6f6c5..77eda460 100644 --- a/internal/controller/cluster_addons_controller.go +++ b/internal/controller/cluster_addons_controller.go @@ -27,12 +27,12 @@ type ReconcileClusterAddonsConfiguration struct { } // NewReconcileClusterAddonsConfiguration returns a new reconcile.Reconciler -func NewReconcileClusterAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { +func NewReconcileClusterAddonsConfiguration(mgr manager.Manager, addonGetterFactory addonGetterFactory, chartStorage chartStorage, addonStorage addonStorage, brokerFacade brokerFacade, docsProvider docsProvider, brokerSyncer brokerSyncer, templateService templateService, tmpDir string, log logrus.FieldLogger) reconcile.Reconciler { return &ReconcileClusterAddonsConfiguration{ log: log.WithField("controller", "cluster-addons"), Client: mgr.GetClient(), - common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, docsProvider, brokerSyncer, brokerFacade, path.Join(tmpDir, "cluster-addon-loader-dst"), log), + common: newControllerCommon(mgr.GetClient(), addonGetterFactory, addonStorage, chartStorage, docsProvider, brokerSyncer, brokerFacade, templateService, path.Join(tmpDir, "cluster-addon-loader-dst"), log), } } diff --git a/internal/controller/cluster_addons_controller_test.go b/internal/controller/cluster_addons_controller_test.go index 91415926..e0dab9b3 100644 --- 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" @@ -50,7 +51,7 @@ func TestReconcileClusterAddonsConfiguration_AddAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -86,7 +87,7 @@ func TestReconcileClusterAddonsConfiguration_AddAddonsProcess_Error(t *testing.T defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -126,7 +127,7 @@ func TestReconcileClusterAddonsConfiguration_UpdateAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -160,7 +161,7 @@ func TestReconcileClusterAddonsConfiguration_UpdateAddonsProcess_ConflictingAddo defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -183,7 +184,7 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess(t *testing.T) { defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -206,7 +207,7 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess_ReconcileOtherA defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -233,7 +234,7 @@ func TestReconcileClusterAddonsConfiguration_DeleteAddonsProcess_Error(t *testin defer ts.assertExpectations() // WHEN - reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, os.TempDir(), spy.NewLogDummy()) + reconciler := NewReconcileClusterAddonsConfiguration(ts.mgr, ts.addonGetterFactory, ts.chartStorage, ts.addonStorage, ts.brokerFacade, ts.docsProvider, ts.brokerSyncer, ts.templateService, os.TempDir(), spy.NewLogDummy()) // THEN result, err := reconciler.Reconcile(reconcile.Request{NamespacedName: types.NamespacedName{Name: fixAddonsCfg.Name}}) @@ -255,6 +256,7 @@ type clusterTestSuite struct { brokerFacade *automock.BrokerFacade docsProvider *automock.DocsProvider brokerSyncer *automock.BrokerSyncer + templateService *repository.Template addonStorage storage.Addon chartStorage storage.Chart } @@ -269,14 +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: repository.NewTemplate(cli), addonStorage: sFact.Addon(), chartStorage: sFact.Chart(), diff --git a/internal/controller/common.go b/internal/controller/common.go index de097867..3c679cb5 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -35,13 +35,14 @@ type common struct { // used to distinguish namespace-scoped and cluster-wide addons configurations namespace internal.Namespace - commonClient commonClient - log logrus.FieldLogger + commonClient commonClient + templateService templateService + log logrus.FieldLogger trace string } -func newControllerCommon(client client.Client, addonGetterFactory addonGetterFactory, addonStorage addonStorage, chartStorage chartStorage, docsProvider docsProvider, brokerSyncer brokerSyncer, brokerFacade brokerFacade, dstPath string, log logrus.FieldLogger) *common { +func newControllerCommon(client client.Client, addonGetterFactory addonGetterFactory, addonStorage addonStorage, chartStorage chartStorage, docsProvider docsProvider, brokerSyncer brokerSyncer, brokerFacade brokerFacade, templateService templateService, dstPath string, log logrus.FieldLogger) *common { return &common{ addonGetterFactory: addonGetterFactory, @@ -54,8 +55,9 @@ func newControllerCommon(client client.Client, addonGetterFactory addonGetterFac docsProvider: docsProvider, protection: protection{}, - namespace: internal.ClusterWide, - commonClient: NewCommonClient(client, log), + namespace: internal.ClusterWide, + templateService: templateService, + commonClient: NewCommonClient(client, log), dstPath: dstPath, log: log, @@ -66,7 +68,7 @@ func newControllerCommon(client client.Client, addonGetterFactory addonGetterFac func (c *common) SetWorkingNamespace(namespace string) { c.namespace = internal.Namespace(namespace) for _, svc := range []NamespacedService{ - c.brokerSyncer, c.brokerFacade, c.docsProvider, c.commonClient, + c.brokerSyncer, c.brokerFacade, c.docsProvider, c.commonClient, c.templateService, } { svc.SetNamespace(namespace) } @@ -168,7 +170,6 @@ func (c *common) OnAdd(addon *internal.CommonAddon, lastStatus v1alpha1.CommonAd } case v1alpha1.AddonsConfigurationReady: saved = c.saveAddons(repositories) - c.statusSnapshot(&addon.Status, repositories) if err = c.updateAddonStatus(addon); err != nil { return errors.Wrap(err, "while update addons configuration status") @@ -249,12 +250,19 @@ func (c *common) loadRepositories(repos []v1alpha1.SpecRepository) *repository.C c.log.Infof("- create addons for %q repository", specRepository.URL) repo := repository.NewAddonsRepository(specRepository.URL) - adds, err := c.createAddons(specRepository.URL) + addonsURL, err := c.templateService.TemplateURL(specRepository) if err != nil { - repo.FetchingError(err) + repo.TemplatingError(err) repositories.AddRepository(repo) + c.log.Errorf("while templating repository URL `%s`: %v", specRepository.URL, err) + continue + } - c.log.Errorf("while creating addons for repository from %q: %s", specRepository.URL, err) + adds, err := c.createAddons(addonsURL) + if err != nil { + repo.FetchingError(err) + repositories.AddRepository(repo) + c.log.Errorf("while creating addons for repository from %s: %v", specRepository.URL, err) continue } @@ -434,7 +442,7 @@ func (c *common) reprocessConfigurationsInConflict(deletedAddonsIDs []string, li if hasConflict := c.isConfigurationInConflict(id, configuration.Status); hasConflict { c.log.Infof("- reprocessing conflicting addons configuration `%s/%s`", configuration.Meta.Namespace, configuration.Meta.Name) if err := c.commonClient.ReprocessRequest(configuration.Meta.Name); err != nil { - return errors.Wrapf(err, "while reprocessing conflicting addons configuration %s", configuration.Meta.Name) + return errors.Wrapf(err, "while reprocessing conflicting addons configuration `%s`", configuration.Meta.Name) } } } diff --git a/internal/controller/common_test.go b/internal/controller/common_test.go index ab720ec8..aa5ec636 100644 --- a/internal/controller/common_test.go +++ b/internal/controller/common_test.go @@ -48,7 +48,7 @@ func TestCommon_OnAdd(t *testing.T) { ts := getClusterTestSuite(t, tc.obj...) defer ts.assertExpectations() common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Exist").Return(false, nil) ts.brokerFacade.On("Create").Return(nil).Once() @@ -109,7 +109,7 @@ func TestCommon_OnDelete(t *testing.T) { t.Run(tn, func(t *testing.T) { ts := getClusterTestSuite(t, tc.obj...) common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Delete").Return(nil).Once() @@ -152,7 +152,7 @@ func TestCommon_OnAdd_NamespaceScoped(t *testing.T) { ts := getTestSuite(t, tc.obj...) defer ts.assertExpectations() common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Exist").Return(false, nil) ts.brokerFacade.On("Create").Return(nil).Once() @@ -217,7 +217,7 @@ func TestCommon_OnDelete_NamespaceScoped(t *testing.T) { t.Run(tn, func(t *testing.T) { ts := getTestSuite(t, tc.obj...) common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Delete").Return(nil).Once() @@ -242,7 +242,7 @@ func TestCommon_PrepareForProcessing(t *testing.T) { t.Run(tn, func(t *testing.T) { ts := getTestSuite(t, tc.obj...) common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Delete").Return(nil).Once() @@ -265,7 +265,7 @@ func TestCommon_PrepareForProcessing_NamespaceScoped(t *testing.T) { t.Run(tn, func(t *testing.T) { ts := getTestSuite(t, tc.obj...) common := newControllerCommon(ts.mgr.GetClient(), ts.addonGetterFactory, ts.addonStorage, ts.chartStorage, - ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, "", logrus.New()) + ts.docsProvider, ts.brokerSyncer, ts.brokerFacade, ts.templateService, "", logrus.New()) ts.brokerFacade.On("Delete").Return(nil).Once() diff --git a/internal/controller/ext.go b/internal/controller/ext.go index e9303cd5..455ac044 100644 --- a/internal/controller/ext.go +++ b/internal/controller/ext.go @@ -4,6 +4,7 @@ import ( "github.com/Masterminds/semver" "github.com/kyma-project/helm-broker/internal" "github.com/kyma-project/helm-broker/internal/addon/provider" + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" "k8s.io/helm/pkg/proto/hapi/chart" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -70,6 +71,12 @@ type commonClient interface { IsNamespaceScoped() bool } +//go:generate mockery -name=templateService -output=automock -outpkg=automock -case=underscore +type templateService interface { + NamespacedService + TemplateURL(repository v1alpha1.SpecRepository) (string, error) +} + type commonReconciler interface { Reconcile(addon *internal.CommonAddon, trace string) (reconcile.Result, error) SetWorkingNamespace(namespace string) diff --git a/internal/controller/repository/repository.go b/internal/controller/repository/repository.go index 2a5a6139..80a687c7 100644 --- a/internal/controller/repository/repository.go +++ b/internal/controller/repository/repository.go @@ -49,3 +49,11 @@ func (ar *Repository) FetchingError(err error) { ar.Repository.Reason = reason ar.Repository.Message = fmt.Sprintf(reason.Message(), err.Error()) } + +// TemplatingError sets StatusRepository as failed with URLTemplatingError as a reason +func (ar *Repository) TemplatingError(err error) { + reason := addonsv1alpha1.RepositoryURLTemplatingError + ar.Failed() + ar.Repository.Reason = reason + ar.Repository.Message = fmt.Sprintf(reason.Message(), err.Error()) +} diff --git a/internal/controller/repository/template.go b/internal/controller/repository/template.go new file mode 100644 index 00000000..f5bf239b --- /dev/null +++ b/internal/controller/repository/template.go @@ -0,0 +1,76 @@ +package repository + +import ( + "context" + "fmt" + "strings" + + "regexp" + + "net/url" + + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "github.com/pkg/errors" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Template contains URL templating from secret logic +type Template struct { + cli client.Client + + namespace string +} + +var reg = regexp.MustCompile("\\{(.*?)\\}") + +// NewTemplate returns a new Template service +func NewTemplate(cli client.Client) *Template { + return &Template{ + cli: cli, + } +} + +// SetNamespace sets service's working namespace +func (t *Template) SetNamespace(namespace string) { + t.namespace = namespace +} + +// TemplateURL returns an URL to the repository with filled template fields +func (t *Template) TemplateURL(repository v1alpha1.SpecRepository) (string, error) { + templateParameters := t.findURLTemplates(repository.URL) + 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 != "" { + fetchNS = repository.SecretRef.Namespace + } + + secret := &v1.Secret{} + err := t.cli.Get(context.Background(), types.NamespacedName{Namespace: fetchNS, Name: repository.SecretRef.Name}, secret) + if err != nil { + return "", errors.Wrapf(err, "while getting secret %s/%s", fetchNS, repository.SecretRef.Name) + } + + result := repository.URL + for _, val := range templateParameters { + fieldName := val[1 : len(val)-1] + tmp, ok := secret.Data[fieldName] + if !ok { + return "", fmt.Errorf("secret does not contain `%s` field", fieldName) + } + result = strings.Replace(result, val, url.QueryEscape(string(tmp)), -1) + } + + return strings.Replace(result, "\n", "", -1), nil +} + +func (t *Template) findURLTemplates(url string) []string { + return reg.FindAllString(url, -1) +} diff --git a/internal/controller/repository/template_test.go b/internal/controller/repository/template_test.go new file mode 100644 index 00000000..da74d921 --- /dev/null +++ b/internal/controller/repository/template_test.go @@ -0,0 +1,176 @@ +package repository + +import ( + "fmt" + "testing" + + "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + secretRef = "test" + + keyIDField = "KEY_ID" + keySecretField = "KEY_SECRET" + + testData = "fix" +) + +func TestTemplate_HappyPath(t *testing.T) { + for tn, tc := range map[string]struct { + specRepository v1alpha1.SpecRepository + objs []runtime.Object + expURL string + }{ + "only-field": { + specRepository: v1alpha1.SpecRepository{ + URL: fmt.Sprintf("{%s}", keyIDField), + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{fixSecret()}, + expURL: fmt.Sprintf("%s", testData), + }, + "one-field": { + specRepository: v1alpha1.SpecRepository{ + URL: fmt.Sprintf("s3://index/path?access_key_id={%s}", keyIDField), + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{fixSecret()}, + expURL: fmt.Sprintf("s3://index/path?access_key_id=%s", testData), + }, + "many-fields": { + specRepository: v1alpha1.SpecRepository{ + URL: fmt.Sprintf("s3://index/path?access_key_id={%s}&access_key_secret={%s}", keyIDField, keySecretField), + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{fixSecret()}, + expURL: fmt.Sprintf("s3://index/path?access_key_id=%s&access_key_secret=%s", testData, testData), + }, + "no-fields": { + specRepository: v1alpha1.SpecRepository{ + URL: "s3://index/path?access_key_id=fix", + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{fixSecret()}, + expURL: "s3://index/path?access_key_id=fix", + }, + "no-fields-no-secret": { + specRepository: v1alpha1.SpecRepository{ + URL: "s3://index/path?access_key_id=fix", + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + expURL: "s3://index/path?access_key_id=fix", + }, + "mixed-fields": { + specRepository: v1alpha1.SpecRepository{ + URL: fmt.Sprintf("s3://index/path?access_key_id={%s}&ref=master&access_key_secret={%s}", keyIDField, keySecretField), + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{fixSecret()}, + expURL: fmt.Sprintf("s3://index/path?access_key_id=%s&ref=master&access_key_secret=%s", testData, testData), + }, + } { + t.Run(tn, func(t *testing.T) { + c := fake.NewFakeClient(tc.objs...) + tmp := NewTemplate(c) + + result, err := tmp.TemplateURL(tc.specRepository) + require.NoError(t, err) + assert.Equal(t, tc.expURL, result) + }) + } + +} + +func TestTemplate_FillNamespaceIfNotProvided(t *testing.T) { + c := fake.NewFakeClient(fixSecret()) + tmp := NewTemplate(c) + + tmp.SetNamespace(secretRef) + result, err := tmp.TemplateURL(v1alpha1.SpecRepository{ + URL: fmt.Sprintf("{%s}", keyIDField), + SecretRef: &v1.SecretReference{ + Name: secretRef, + }, + }) + require.NoError(t, err) + assert.Equal(t, testData, result) +} + +func TestTemplate_Error(t *testing.T) { + for tn, tc := range map[string]struct { + specRepository v1alpha1.SpecRepository + objs []runtime.Object + errMsg string + }{ + "missing-secret": { + specRepository: v1alpha1.SpecRepository{ + URL: "access_key_id={KEY_NAME_FROM_SECRET_PLACEHOLDER}", + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + errMsg: "while getting secret test/test: secrets \"test\" not found", + }, + "missing-field": { + specRepository: v1alpha1.SpecRepository{ + URL: "access_key_id={KEY_NAME_FROM_SECRET_PLACEHOLDER}", + SecretRef: &v1.SecretReference{ + Name: secretRef, + Namespace: secretRef, + }, + }, + objs: []runtime.Object{ + fixSecret(), + }, + errMsg: "secret does not contain `KEY_NAME_FROM_SECRET_PLACEHOLDER` field", + }, + } { + t.Run(tn, func(t *testing.T) { + c := fake.NewFakeClient(tc.objs...) + tmp := NewTemplate(c) + + _, err := tmp.TemplateURL(tc.specRepository) + require.EqualError(t, err, tc.errMsg) + }) + } + +} + +func fixSecret() *v1.Secret { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretRef, + Namespace: secretRef, + }, + Data: map[string][]byte{ + keyIDField: []byte(testData), + keySecretField: []byte(testData), + }, + } +} diff --git a/internal/controller/setup.go b/internal/controller/setup.go index 8b5ab19f..69ac66f6 100644 --- a/internal/controller/setup.go +++ b/internal/controller/setup.go @@ -11,6 +11,7 @@ import ( "github.com/kyma-project/helm-broker/internal/config" "github.com/kyma-project/helm-broker/internal/controller/broker" "github.com/kyma-project/helm-broker/internal/controller/docs" + "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/kyma/components/cms-controller-manager/pkg/apis/cms/v1alpha1" @@ -57,6 +58,8 @@ func SetupAndStartController(cfg *rest.Config, ctrCfg *config.ControllerConfig, sbFacade := broker.NewBrokersFacade(mgr.GetClient(), ctrCfg.Namespace, ctrCfg.ServiceName, lg) csbFacade := broker.NewClusterBrokersFacade(mgr.GetClient(), ctrCfg.Namespace, ctrCfg.ServiceName, ctrCfg.ClusterServiceBrokerName, lg) + templateService := repository.NewTemplate(mgr.GetClient()) + gitGetterFactory := provider.GitGetterConfiguration{Cli: uploadClient, TmpDir: ctrCfg.TmpDir} allowedGetters := map[string]provider.Provider{ "git": gitGetterFactory.NewGit, @@ -74,12 +77,12 @@ func SetupAndStartController(cfg *rest.Config, ctrCfg *config.ControllerConfig, // Creating controllers lg.Info("Setting up controller") - acReconcile := NewReconcileAddonsConfiguration(mgr, addonGetterFactory, sFact.Chart(), sFact.Addon(), sbFacade, dtProvider, sbSyncer, ctrCfg.TmpDir, lg) + acReconcile := NewReconcileAddonsConfiguration(mgr, addonGetterFactory, sFact.Chart(), sFact.Addon(), sbFacade, dtProvider, sbSyncer, templateService, ctrCfg.TmpDir, lg) acController := NewAddonsConfigurationController(acReconcile) err = acController.Start(mgr) fatalOnError(err, "unable to start AddonsConfigurationController") - cacReconcile := NewReconcileClusterAddonsConfiguration(mgr, addonGetterFactory, sFact.Chart(), sFact.Addon(), csbFacade, cdtProvider, csbSyncer, ctrCfg.TmpDir, lg) + cacReconcile := NewReconcileClusterAddonsConfiguration(mgr, addonGetterFactory, sFact.Chart(), sFact.Addon(), csbFacade, cdtProvider, csbSyncer, templateService, ctrCfg.TmpDir, lg) cacController := NewClusterAddonsConfigurationController(cacReconcile) err = cacController.Start(mgr) fatalOnError(err, "unable to start ClusterAddonsConfigurationController") diff --git a/internal/model.go b/internal/model.go index 915f6ea2..25bf01f4 100644 --- a/internal/model.go +++ b/internal/model.go @@ -11,6 +11,7 @@ import ( "github.com/kyma-project/helm-broker/pkg/apis/addons/v1alpha1" cms "github.com/kyma-project/kyma/components/cms-controller-manager/pkg/apis/cms/v1alpha1" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/helm/pkg/proto/hapi/chart" ) @@ -172,6 +173,7 @@ type Addon struct { Status v1alpha1.AddonStatus Reason v1alpha1.AddonStatusReason Message string + SecretRef corev1.SecretReference } // CommonAddon holds common addon configuration structs diff --git a/pkg/apis/addons/v1alpha1/common.go b/pkg/apis/addons/v1alpha1/common.go index 1908a1f2..b0827c38 100644 --- a/pkg/apis/addons/v1alpha1/common.go +++ b/pkg/apis/addons/v1alpha1/common.go @@ -43,8 +43,8 @@ const ( // SpecRepository define the addon repository type SpecRepository struct { - URL string `json:"url"` - SecretRef v1.SecretReference `json:"secretRef"` + URL string `json:"url"` + SecretRef *v1.SecretReference `json:"secretRef,omitempty"` } // CommonAddonsConfigurationSpec defines the desired state of (Cluster)AddonsConfiguration diff --git a/pkg/apis/addons/v1alpha1/reason.go b/pkg/apis/addons/v1alpha1/reason.go index 5342e77f..08a787a0 100644 --- a/pkg/apis/addons/v1alpha1/reason.go +++ b/pkg/apis/addons/v1alpha1/reason.go @@ -37,7 +37,8 @@ func (r AddonStatusReason) Message() string { type RepositoryStatusReason string const ( - RepositoryURLFetchingError RepositoryStatusReason = "FetchingIndexError" + RepositoryURLFetchingError RepositoryStatusReason = "FetchingIndexError" + RepositoryURLTemplatingError RepositoryStatusReason = "TemplatingURLError" ) func (r RepositoryStatusReason) String() string { diff --git a/pkg/apis/addons/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/addons/v1alpha1/zz_generated.deepcopy.go index a1580363..41c2521b 100644 --- a/pkg/apis/addons/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/addons/v1alpha1/zz_generated.deepcopy.go @@ -5,6 +5,7 @@ package v1alpha1 import ( + v1 "k8s.io/api/core/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -14,7 +15,7 @@ func (in *Addon) DeepCopyInto(out *Addon) { return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AddonWithCharts. +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Addon. func (in *Addon) DeepCopy() *Addon { if in == nil { return nil @@ -220,7 +221,9 @@ func (in *CommonAddonsConfigurationSpec) DeepCopyInto(out *CommonAddonsConfigura if in.Repositories != nil { in, out := &in.Repositories, &out.Repositories *out = make([]SpecRepository, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -265,7 +268,11 @@ func (in *CommonAddonsConfigurationStatus) DeepCopy() *CommonAddonsConfiguration // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SpecRepository) DeepCopyInto(out *SpecRepository) { *out = *in - out.SecretRef = in.SecretRef + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = new(v1.SecretReference) + **out = **in + } return } diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 7a9d1c24..40810931 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -9,8 +9,6 @@ import ( ) func TestHttpBasicAuth(t *testing.T) { - // todo: uncomment after templating is done - t.Skip() // given suite := newTestSuite(t, true, true) defer suite.tearDown() diff --git a/test/integration/suite_test.go b/test/integration/suite_test.go index 65c8002d..eacc11f2 100644 --- 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" ) @@ -369,7 +369,7 @@ func (ts *testSuite) createSpecRepositories(urls []string, repoKind string, repo func WithSecretReference(namespace, name string) func(r *v1alpha1.SpecRepository) { return func(r *v1alpha1.SpecRepository) { - r.SecretRef = corev1.SecretReference{ + r.SecretRef = &corev1.SecretReference{ Namespace: namespace, Name: name, } @@ -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) } }