diff --git a/cmd/kubeops/internal/handler/handler.go b/cmd/kubeops/internal/handler/handler.go index de077eb6b44..3152a7e6325 100644 --- a/cmd/kubeops/internal/handler/handler.go +++ b/cmd/kubeops/internal/handler/handler.go @@ -52,13 +52,13 @@ type Options struct { // Config represents data needed by each handler to be able to create Helm 3 actions. // It cannot be created without a bearer token, so a new one must be created upon each HTTP request. type Config struct { - ActionConfig *action.Configuration - Options Options - KubeHandler kube.AuthHandler - Resolver handlerutil.ResolverFactory - Cluster string - Token string - userClientSet kubernetes.Interface + ActionConfig *action.Configuration + Options Options + KubeHandler kube.AuthHandler + ChartClientFactory chartUtils.ChartClientFactoryInterface + Cluster string + Token string + userClientSet kubernetes.Interface } // WithHandlerConfig takes a dependentHandler and creates a regular (WithParams) handler that, @@ -110,13 +110,13 @@ func WithHandlerConfig(storageForDriver agent.StorageForDriver, options Options) } cfg := Config{ - Options: options, - ActionConfig: actionConfig, - KubeHandler: kubeHandler, - Cluster: cluster, - Token: token, - Resolver: &handlerutil.ClientResolver{}, - userClientSet: userKubeClient, + Options: options, + ActionConfig: actionConfig, + KubeHandler: kubeHandler, + Cluster: cluster, + Token: token, + ChartClientFactory: &chartUtils.ChartClientFactory{}, + userClientSet: userKubeClient, } f(cfg, w, req, params) } @@ -191,7 +191,7 @@ func CreateRelease(cfg Config, w http.ResponseWriter, req *http.Request, params chartDetails, appRepo, caCertSecret, authSecret, - cfg.Resolver.New(appRepo.Spec.Type, cfg.Options.UserAgent), + cfg.ChartClientFactory.New(appRepo.Spec.Type, cfg.Options.UserAgent), ) if err != nil { returnErrMessage(err, w) @@ -245,7 +245,7 @@ func upgradeRelease(cfg Config, w http.ResponseWriter, req *http.Request, params chartDetails, appRepo, caCertSecret, authSecret, - cfg.Resolver.New(appRepo.Spec.Type, cfg.Options.UserAgent), + cfg.ChartClientFactory.New(appRepo.Spec.Type, cfg.Options.UserAgent), ) registrySecrets, err := chartUtils.RegistrySecretsPerDomain(req.Context(), appRepo.Spec.DockerRegistrySecrets, appRepo.Namespace, cfg.userClientSet) if err != nil { diff --git a/cmd/kubeops/internal/handler/handler_test.go b/cmd/kubeops/internal/handler/handler_test.go index 77b746dc220..f946daf347f 100644 --- a/cmd/kubeops/internal/handler/handler_test.go +++ b/cmd/kubeops/internal/handler/handler_test.go @@ -13,7 +13,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1" - fakeHandlerUtils "github.com/kubeapps/kubeapps/pkg/handlerutil/fake" + fakeChartUtils "github.com/kubeapps/kubeapps/pkg/chart/fake" kubeappsKube "github.com/kubeapps/kubeapps/pkg/kube" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" @@ -57,7 +57,7 @@ func newConfigFixture(t *testing.T, k *kubefake.FailingKubeClient) *Config { }, }, }, - Resolver: &fakeHandlerUtils.ClientResolver{}, + ChartClientFactory: &fakeChartUtils.ChartClientFactory{}, Options: Options{ ListLimit: defaultListLimit, }, diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 70a558bc207..8c8810d388f 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -189,7 +189,7 @@ func TestCreateReleases(t *testing.T) { // Initialize environment for test actionConfig := newActionConfigFixture(t) makeReleases(t, actionConfig, tc.existingReleases) - fakechart := chartFake.Client{} + fakechart := chartFake.ChartClient{} ch, _ := fakechart.GetChart(&kubechart.Details{ ChartName: tc.chartName, }, "") @@ -678,7 +678,7 @@ func TestUpgradeRelease(t *testing.T) { t.Run(tc.description, func(t *testing.T) { cfg := newActionConfigFixture(t) makeReleases(t, cfg, tc.releases) - fakechart := chartFake.Client{} + fakechart := chartFake.ChartClient{} ch, _ := fakechart.GetChart(&kubechart.Details{ ChartName: tc.chartName, }, "") diff --git a/pkg/chart/chart.go b/pkg/chart/chart.go index ad775c93e5a..b79f840a5b7 100644 --- a/pkg/chart/chart.go +++ b/pkg/chart/chart.go @@ -82,34 +82,34 @@ type Details struct { // LoadHelmChart returns a helm3 Chart struct from an IOReader type LoadHelmChart func(in io.Reader) (*helm3chart.Chart, error) -// Resolver for exposed funcs -type Resolver interface { - InitClient(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error +// ChartClient for exposed funcs +type ChartClient interface { + Init(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error GetChart(details *Details, repoURL string) (*helm3chart.Chart, error) } -// Client struct contains the clients required to retrieve charts info -type Client struct { +// HelmRepoClient struct contains the clients required to retrieve charts info +type HelmRepoClient struct { userAgent string netClient httpclient.Client } // NewChartClient returns a new ChartClient -func NewChartClient(userAgent string) Resolver { - return &Client{ +func NewChartClient(userAgent string) ChartClient { + return &HelmRepoClient{ userAgent: userAgent, } } -// OCIClient struct contains the clients required to retrieve charts info from an OCI registry -type OCIClient struct { +// OCIRepoClient struct contains the clients required to retrieve charts info from an OCI registry +type OCIRepoClient struct { userAgent string puller helm.ChartPuller } // NewOCIClient returns a new OCIClient -func NewOCIClient(userAgent string) Resolver { - return &OCIClient{ +func NewOCIClient(userAgent string) ChartClient { + return &OCIRepoClient{ userAgent: userAgent, } } @@ -313,9 +313,9 @@ func GetAppRepoAndRelatedSecrets(appRepoName, appRepoNamespace string, handler k return appRepo, caCertSecret, authSecret, nil } -// InitClient returns an HTTP client based on the chart details loading a +// Init initialises the HTTP client based on the chart details loading a // custom CA if provided (as a secret) -func (c *Client) InitClient(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { +func (c *HelmRepoClient) Init(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { var err error c.netClient, err = kube.InitNetClient(appRepo, caCertSecret, authSecret, http.Header{"User-Agent": []string{c.userAgent}}) return err @@ -323,9 +323,9 @@ func (c *Client) InitClient(appRepo *appRepov1.AppRepository, caCertSecret *core // GetChart retrieves and loads a Chart from a registry in both // v2 and v3 formats. -func (c *Client) GetChart(details *Details, repoURL string) (*helm3chart.Chart, error) { +func (c *HelmRepoClient) GetChart(details *Details, repoURL string) (*helm3chart.Chart, error) { if c.netClient == nil { - return nil, fmt.Errorf("unable to retrieve chart, InitClient should be called first") + return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } var chart *helm3chart.Chart indexURL := strings.TrimSuffix(strings.TrimSpace(repoURL), "/") + "/index.yaml" @@ -381,10 +381,10 @@ func RegistrySecretsPerDomain(ctx context.Context, appRepoSecrets []string, name return secretsPerDomain, nil } -// InitClient returns an HTTP client based on the chart details loading a +// Init initialises the HTTP client based on the chart details loading a // custom CA if provided (as a secret) // TODO(andresmgot): Using a custom CA cert is not supported by ORAS (neither helm), only using the insecure flag -func (c *OCIClient) InitClient(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { +func (c *OCIRepoClient) Init(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { var err error headers := http.Header{ "User-Agent": []string{c.userAgent}, @@ -407,9 +407,9 @@ func (c *OCIClient) InitClient(appRepo *appRepov1.AppRepository, caCertSecret *c } // GetChart retrieves and loads a Chart from a OCI registry -func (c *OCIClient) GetChart(details *Details, repoURL string) (*helm3chart.Chart, error) { +func (c *OCIRepoClient) GetChart(details *Details, repoURL string) (*helm3chart.Chart, error) { if c.puller == nil { - return nil, fmt.Errorf("unable to retrieve chart, InitClient should be called first") + return nil, fmt.Errorf("unable to retrieve chart, Init should be called first") } url, err := url.ParseRequestURI(strings.TrimSpace(repoURL)) if err != nil { @@ -424,3 +424,28 @@ func (c *OCIClient) GetChart(details *Details, repoURL string) (*helm3chart.Char return helm3loader.LoadArchive(chartBuffer) } + +// ChartClientFactoryInterface defines how a ChartClientFactory implementation +// can return a chart client. +// +// This can be implemented with a fake for tests. +type ChartClientFactoryInterface interface { + New(repoType, userAgent string) ChartClient +} + +// ChartClientFactory provides a real implementation of the ChartClientFactory interface +// returning either an OCI repository client or a traditional helm repository chart client. +type ChartClientFactory struct{} + +// New for ClientResolver +func (c *ChartClientFactory) New(repoType, userAgent string) ChartClient { + var client ChartClient + switch repoType { + case "oci": + client = NewOCIClient(userAgent) + break + default: + client = NewChartClient(userAgent) + } + return client +} diff --git a/pkg/chart/chart_test.go b/pkg/chart/chart_test.go index dcdade063c5..4efe0dc0980 100644 --- a/pkg/chart/chart_test.go +++ b/pkg/chart/chart_test.go @@ -546,7 +546,7 @@ func TestGetChart(t *testing.T) { } t.Run(tc.name, func(t *testing.T) { httpClient := newHTTPClient(repoURL, []Details{target}, tc.userAgent) - chUtils := Client{ + chUtils := HelmRepoClient{ userAgent: tc.userAgent, } chUtils.netClient = httpClient @@ -593,7 +593,7 @@ func TestGetChart(t *testing.T) { t.Run("it should fail if the netClient is not instantiated", func(t *testing.T) { cli := NewChartClient("") _, err := cli.GetChart(nil, "") - assert.Err(t, fmt.Errorf("unable to retrieve chart, InitClient should be called first"), err) + assert.Err(t, fmt.Errorf("unable to retrieve chart, Init should be called first"), err) }) } @@ -848,8 +848,8 @@ func TestGetRegistrySecretsPerDomain(t *testing.T) { func TestOCIClient(t *testing.T) { t.Run("InitClient - Creates puller with User-Agent header", func(t *testing.T) { cli := NewOCIClient("foo") - cli.InitClient(&appRepov1.AppRepository{}, &corev1.Secret{}, &corev1.Secret{}) - helmtest.CheckHeader(t, cli.(*OCIClient).puller, "User-Agent", "foo") + cli.Init(&appRepov1.AppRepository{}, &corev1.Secret{}, &corev1.Secret{}) + helmtest.CheckHeader(t, cli.(*OCIRepoClient).puller, "User-Agent", "foo") }) t.Run("InitClient - Creates puller with Authorization", func(t *testing.T) { @@ -871,8 +871,8 @@ func TestOCIClient(t *testing.T) { "custom-secret-key": []byte("Basic Auth"), }, } - cli.InitClient(appRepo, &corev1.Secret{}, authSecret) - helmtest.CheckHeader(t, cli.(*OCIClient).puller, "Authorization", "Basic Auth") + cli.Init(appRepo, &corev1.Secret{}, authSecret) + helmtest.CheckHeader(t, cli.(*OCIRepoClient).puller, "Authorization", "Basic Auth") }) t.Run("InitClient - Creates puller with Docker Creds Authorization", func(t *testing.T) { @@ -894,23 +894,23 @@ func TestOCIClient(t *testing.T) { ".dockerconfigjson": []byte(`{"auths":{"foo":{"username":"foo","password":"bar"}}}`), }, } - err := cli.InitClient(appRepo, &corev1.Secret{}, authSecret) + err := cli.Init(appRepo, &corev1.Secret{}, authSecret) if err != nil { t.Fatalf("Unexpected error: %v", err) } // Authorization: Basic base64('foo:bar') - helmtest.CheckHeader(t, cli.(*OCIClient).puller, "Authorization", "Basic Zm9vOmJhcg==") + helmtest.CheckHeader(t, cli.(*OCIRepoClient).puller, "Authorization", "Basic Zm9vOmJhcg==") }) t.Run("GetChart - Fails if the puller has not been instantiated", func(t *testing.T) { cli := NewOCIClient("foo") _, err := cli.GetChart(nil, "") - assert.Err(t, fmt.Errorf("unable to retrieve chart, InitClient should be called first"), err) + assert.Err(t, fmt.Errorf("unable to retrieve chart, Init should be called first"), err) }) t.Run("GetChart - Fails if the URL is not valid", func(t *testing.T) { cli := NewOCIClient("foo") - cli.(*OCIClient).puller = &helmfake.OCIPuller{} + cli.(*OCIRepoClient).puller = &helmfake.OCIPuller{} _, err := cli.GetChart(nil, "foo") if !strings.Contains(err.Error(), "invalid URI for request") { t.Errorf("Unexpected error %v", err) @@ -921,7 +921,7 @@ func TestOCIClient(t *testing.T) { cli := NewOCIClient("foo") data, err := ioutil.ReadFile("./testdata/nginx-5.1.1-apiVersionV2.tgz") assert.NoErr(t, err) - cli.(*OCIClient).puller = &helmfake.OCIPuller{ + cli.(*OCIRepoClient).puller = &helmfake.OCIPuller{ ExpectedName: "foo/bar/nginx:5.1.1", Content: map[string]*bytes.Buffer{"5.1.1": bytes.NewBuffer(data)}, } diff --git a/pkg/chart/fake/chart.go b/pkg/chart/fake/chart.go index 7e58e6a431e..d0cabf671d4 100644 --- a/pkg/chart/fake/chart.go +++ b/pkg/chart/fake/chart.go @@ -24,11 +24,11 @@ import ( "sigs.k8s.io/yaml" ) -// Client implements Resolver inteface -type Client struct{} +// ChartClient implements Resolver inteface +type ChartClient struct{} // GetChart fake -func (f *Client) GetChart(details *chartUtils.Details, repoURL string) (*chart3.Chart, error) { +func (f *ChartClient) GetChart(details *chartUtils.Details, repoURL string) (*chart3.Chart, error) { vals, err := getValues([]byte(details.Values)) if err != nil { return nil, err @@ -50,7 +50,15 @@ func getValues(raw []byte) (map[string]interface{}, error) { return values, nil } -// InitClient fake -func (f *Client) InitClient(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { +// Init fake +func (f *ChartClient) Init(appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret) error { return nil } + +// ChartClientFactory is a fake implementation of the ChartClientFactory interface. +type ChartClientFactory struct{} + +// New returns a fake ChartClient +func (c *ChartClientFactory) New(repoType, userAgent string) chartUtils.ChartClient { + return &ChartClient{} +} diff --git a/pkg/handlerutil/fake/handlerutil.go b/pkg/handlerutil/fake/handlerutil.go deleted file mode 100644 index 6a776977569..00000000000 --- a/pkg/handlerutil/fake/handlerutil.go +++ /dev/null @@ -1,14 +0,0 @@ -package fake - -import ( - chartUtils "github.com/kubeapps/kubeapps/pkg/chart" - fakeChart "github.com/kubeapps/kubeapps/pkg/chart/fake" -) - -// ClientResolver implements ResolverFactory -type ClientResolver struct{} - -// New for ClientResolver -func (c *ClientResolver) New(repoType, userAgent string) chartUtils.Resolver { - return &fakeChart.Client{} -} diff --git a/pkg/handlerutil/handlerutil.go b/pkg/handlerutil/handlerutil.go index cfb40f8c02d..83d6e7f1a39 100644 --- a/pkg/handlerutil/handlerutil.go +++ b/pkg/handlerutil/handlerutil.go @@ -83,34 +83,13 @@ func ParseRequest(req *http.Request) (*chartUtils.Details, error) { return chartDetails, nil } -// ResolverFactory interface to return a resolver -type ResolverFactory interface { - New(repoType, userAgent string) chartUtils.Resolver -} - -// ClientResolver implements ResolverFactory -type ClientResolver struct{} - -// New for ClientResolver -func (c *ClientResolver) New(repoType, userAgent string) chartUtils.Resolver { - var cu chartUtils.Resolver - switch repoType { - case "oci": - cu = chartUtils.NewOCIClient(userAgent) - break - default: - cu = chartUtils.NewChartClient(userAgent) - } - return cu -} - // GetChart retrieves a chart -func GetChart(chartDetails *chartUtils.Details, appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret, resolver chartUtils.Resolver) (*chart.Chart, error) { - err := resolver.InitClient(appRepo, caCertSecret, authSecret) +func GetChart(chartDetails *chartUtils.Details, appRepo *appRepov1.AppRepository, caCertSecret *corev1.Secret, authSecret *corev1.Secret, chartClient chartUtils.ChartClient) (*chart.Chart, error) { + err := chartClient.Init(appRepo, caCertSecret, authSecret) if err != nil { return nil, err } - ch, err := resolver.GetChart(chartDetails, appRepo.Spec.URL) + ch, err := chartClient.GetChart(chartDetails, appRepo.Spec.URL) if err != nil { return nil, err }