From a8741140a34f1ca86d3d1a953399c5755ee5f2a7 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 12 Feb 2021 15:55:23 -0500 Subject: [PATCH 1/7] Implement retrieving plugins from URI and remote registry Signed-off-by: Angel Misevski --- pkg/library/flatten/flatten.go | 2 + pkg/library/flatten/network/devfile.go | 40 ++++++++++++++++++++ pkg/library/flatten/network/fetch.go | 51 ++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 pkg/library/flatten/network/devfile.go create mode 100644 pkg/library/flatten/network/fetch.go diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index c6323d118..3b6530f1f 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -15,6 +15,7 @@ package flatten import ( "context" "fmt" + "net/http" "github.com/devfile/devworkspace-operator/pkg/library/flatten/web_terminal" @@ -32,6 +33,7 @@ type ResolverTools struct { Context context.Context K8sClient client.Client InternalRegistry registry.InternalRegistry + HttpClient http.Client } // ResolveDevWorkspace takes a devworkspace and returns a "resolved" version of it -- i.e. one where all plugins and parents diff --git a/pkg/library/flatten/network/devfile.go b/pkg/library/flatten/network/devfile.go new file mode 100644 index 000000000..84392b8a0 --- /dev/null +++ b/pkg/library/flatten/network/devfile.go @@ -0,0 +1,40 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package network + +import ( + "fmt" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + devfilev2 "github.com/devfile/api/v2/pkg/devfile" +) + +const ( + SupportedDevfileSchemaVersion = "2.0.0" // TODO what should this actually be at this point? +) + +type Devfile struct { + devfilev2.DevfileHeader + dw.DevWorkspaceTemplateSpec +} + +func ConvertDevfileToDevWorkspaceTemplate(devfile *Devfile) (*dw.DevWorkspaceTemplate, error) { + if devfile.SchemaVersion != SupportedDevfileSchemaVersion { + return nil, fmt.Errorf("could not process devfile: supported schemaVersion is %s", SupportedDevfileSchemaVersion) + } + dwt := &dw.DevWorkspaceTemplate{} + dwt.Spec = devfile.DevWorkspaceTemplateSpec + dwt.Name = devfile.Metadata.Name // TODO: Handle additional devfile metadata once those changes are pulled in to this repo + + return dwt, nil +} diff --git a/pkg/library/flatten/network/fetch.go b/pkg/library/flatten/network/fetch.go new file mode 100644 index 000000000..f738f82c0 --- /dev/null +++ b/pkg/library/flatten/network/fetch.go @@ -0,0 +1,51 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package network + +import ( + "fmt" + "io/ioutil" + "net/http" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "sigs.k8s.io/yaml" +) + +func FetchDevWorkspaceTemplate(location string, httpClient *http.Client) (*dw.DevWorkspaceTemplateSpec, map[string]string, error) { + resp, err := httpClient.Get(location) + if err != nil { + return nil, nil, fmt.Errorf("failed to fetch file from %s: %w", location, err) + } + defer resp.Body.Close() // ignoring error because what would we even do? + if resp.StatusCode != http.StatusOK { + return nil, nil, fmt.Errorf("could not fetch file from %s: got status %d", location, resp.StatusCode) + } + bytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, nil, fmt.Errorf("could not read data from %s: %w", location, err) + } + + // Assume we're getting a devfile, not a DevWorkspaceTemplate (TODO: Detect type and handle both?) + devfile := &Devfile{} + err = yaml.Unmarshal(bytes, devfile) + if err != nil { + return nil, nil, fmt.Errorf("could not unmarshal devfile from response: %w", err) + } + + dwt, err := ConvertDevfileToDevWorkspaceTemplate(devfile) + if err != nil { + return nil, nil, fmt.Errorf("failed to convert devfile to DevWorkspaceTemplate: %s", err) + } + + return &dwt.Spec, dwt.Labels, nil +} From cd0da2af2d2d6823172acacb0602d7562acae8fc Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 16 Feb 2021 15:03:52 -0500 Subject: [PATCH 2/7] Add support for plugins defined in registries and via URI Signed-off-by: Angel Misevski --- .../workspace/devworkspace_controller.go | 2 + pkg/library/flatten/flatten.go | 32 +++- pkg/library/flatten/flatten_test.go | 162 ++---------------- pkg/library/flatten/testutil/common.go | 106 ++++++++++++ .../flatten/testutil/internalRegistry.go | 41 +++++ pkg/library/flatten/testutil/k8sClient.go | 50 ++++++ 6 files changed, 240 insertions(+), 153 deletions(-) create mode 100644 pkg/library/flatten/testutil/common.go create mode 100644 pkg/library/flatten/testutil/internalRegistry.go create mode 100644 pkg/library/flatten/testutil/k8sClient.go diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index fd2d47460..7d8fdab1a 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -16,6 +16,7 @@ import ( "context" "errors" "fmt" + "net/http" "strings" "time" @@ -182,6 +183,7 @@ func (r *DevWorkspaceReconciler) Reconcile(req ctrl.Request) (reconcileResult ct Context: ctx, K8sClient: r.Client, InternalRegistry: ®istry.InternalRegistryImpl{}, + HttpClient: http.DefaultClient, } flattenedWorkspace, err := flatten.ResolveDevWorkspace(workspace.Spec.Template, flattenHelpers) if err != nil { diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index 3b6530f1f..b682142c3 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -16,6 +16,10 @@ import ( "context" "fmt" "net/http" + "net/url" + "path" + + "github.com/devfile/devworkspace-operator/pkg/library/flatten/network" "github.com/devfile/devworkspace-operator/pkg/library/flatten/web_terminal" @@ -33,7 +37,7 @@ type ResolverTools struct { Context context.Context K8sClient client.Client InternalRegistry registry.InternalRegistry - HttpClient http.Client + HttpClient *http.Client } // ResolveDevWorkspace takes a devworkspace and returns a "resolved" version of it -- i.e. one where all plugins and parents @@ -117,7 +121,7 @@ func resolvePluginComponent( } resolvedPlugin, pluginMeta, err = resolvePluginComponentByKubernetesReference(name, plugin, tooling) case plugin.Uri != "": - err = fmt.Errorf("failed to resolve plugin %s: only plugins specified by kubernetes reference or id are supported", name) + resolvedPlugin, pluginMeta, err = resolvePluginComponentByURI(name, plugin, tooling) case plugin.Id != "": resolvedPlugin, pluginMeta, err = resolvePluginComponentById(name, plugin, tooling) default: @@ -181,5 +185,27 @@ func resolvePluginComponentById( return &pluginDWT.Spec, pluginDWT.Labels, nil } - return nil, nil, fmt.Errorf("non-internal plugins not supported") + pluginURL, err := url.Parse(plugin.RegistryUrl) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse registry URL for plugin %s: %w", name, err) + } + pluginURL.Path = path.Join(pluginURL.Path, "plugins", plugin.Id) + + dwt, labels, err := network.FetchDevWorkspaceTemplate(pluginURL.String(), tools.HttpClient) + if err != nil { + return nil, nil, fmt.Errorf("failed to resolve plugin %s from registry %s: %w", name, plugin.RegistryUrl, err) + } + return dwt, labels, nil +} + +func resolvePluginComponentByURI( + name string, + plugin *devworkspace.PluginComponent, + tools ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, pluginLabels map[string]string, err error) { + + dwt, labels, err := network.FetchDevWorkspaceTemplate(plugin.Uri, tools.HttpClient) + if err != nil { + return nil, nil, fmt.Errorf("failed to resolve plugin %s by URI: %w", name, err) + } + return dwt, labels, nil } diff --git a/pkg/library/flatten/flatten_test.go b/pkg/library/flatten/flatten_test.go index a5b98c299..2a82e639f 100644 --- a/pkg/library/flatten/flatten_test.go +++ b/pkg/library/flatten/flatten_test.go @@ -14,161 +14,23 @@ package flatten import ( "context" - "errors" - "fmt" - "io/ioutil" - "path/filepath" - "strings" "testing" - "github.com/devfile/devworkspace-operator/pkg/config" - corev1 "k8s.io/api/core/v1" + "github.com/devfile/devworkspace-operator/pkg/library/flatten/testutil" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" - k8sErrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/yaml" - - devworkspace "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" ) -var workspaceTemplateDiffOpts = cmp.Options{ - cmpopts.SortSlices(func(a, b devworkspace.Component) bool { - return strings.Compare(a.Key(), b.Key()) > 0 - }), - cmpopts.SortSlices(func(a, b string) bool { - return strings.Compare(a, b) > 0 - }), - // TODO: Devworkspace overriding results in empty []string instead of nil - cmpopts.IgnoreFields(devworkspace.WorkspaceEvents{}, "PostStart", "PreStop", "PostStop"), -} - -var testControllerCfg = &corev1.ConfigMap{ - Data: map[string]string{ - "devworkspace.default_dockerimage.redhat-developer.web-terminal": ` -name: default-web-terminal-tooling -container: - name: default-web-terminal-tooling-container - image: test-image -`, - }, -} - -func setupControllerCfg() { - config.SetupConfigForTesting(testControllerCfg) -} - -type testCase struct { - Name string `json:"name"` - Input testInput `json:"input"` - Output testOutput `json:"output"` -} - -type testInput struct { - Workspace devworkspace.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` - // Plugins is a map of plugin "name" to devworkspace template; namespace is ignored. - Plugins map[string]devworkspace.DevWorkspaceTemplate `json:"plugins,omitempty"` - // Errors is a map of plugin name to the error that should be returned when attempting to retrieve it. - Errors map[string]testPluginError `json:"errors,omitempty"` -} - -type testPluginError struct { - IsNotFound bool `json:"isNotFound"` - Message string `json:"message"` -} - -type testOutput struct { - Workspace *devworkspace.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` - ErrRegexp *string `json:"errRegexp,omitempty"` -} - -type fakeK8sClient struct { - client.Client // To satisfy interface; override all used methods - plugins map[string]devworkspace.DevWorkspaceTemplate - errors map[string]testPluginError -} - -func (client *fakeK8sClient) Get(_ context.Context, namespacedName client.ObjectKey, obj runtime.Object) error { - template, ok := obj.(*devworkspace.DevWorkspaceTemplate) - if !ok { - return fmt.Errorf("Called Get() in fake client with non-DevWorkspaceTemplate") - } - if plugin, ok := client.plugins[namespacedName.Name]; ok { - *template = plugin - return nil - } - if err, ok := client.errors[namespacedName.Name]; ok { - if err.IsNotFound { - return k8sErrors.NewNotFound(schema.GroupResource{}, namespacedName.Name) - } else { - return errors.New(err.Message) - } - } - return fmt.Errorf("test does not define an entry for plugin %s", namespacedName.Name) -} - -type fakeInternalRegistry struct { - Plugins map[string]devworkspace.DevWorkspaceTemplate - Errors map[string]testPluginError -} - -func (reg *fakeInternalRegistry) IsInInternalRegistry(pluginID string) bool { - _, pluginOk := reg.Plugins[pluginID] - _, errOk := reg.Errors[pluginID] - return pluginOk || errOk -} - -func (reg *fakeInternalRegistry) ReadPluginFromInternalRegistry(pluginID string) (*devworkspace.DevWorkspaceTemplate, error) { - if plugin, ok := reg.Plugins[pluginID]; ok { - return &plugin, nil - } - if err, ok := reg.Errors[pluginID]; ok { - return nil, errors.New(err.Message) - } - return nil, fmt.Errorf("test does not define entry for plugin %s", pluginID) -} - -func loadTestCaseOrPanic(t *testing.T, testFilepath string) testCase { - bytes, err := ioutil.ReadFile(testFilepath) - if err != nil { - t.Fatal(err) - } - var test testCase - if err := yaml.Unmarshal(bytes, &test); err != nil { - t.Fatal(err) - } - t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) - return test -} - -func loadAllTestsOrPanic(t *testing.T, fromDir string) []testCase { - files, err := ioutil.ReadDir(fromDir) - if err != nil { - t.Fatal(err) - } - var tests []testCase - for _, file := range files { - if file.IsDir() { - continue - } - tests = append(tests, loadTestCaseOrPanic(t, filepath.Join(fromDir, file.Name()))) - } - return tests -} - func TestResolveDevWorkspaceKubernetesReference(t *testing.T) { - tests := loadAllTestsOrPanic(t, "testdata/k8s-ref") + tests := testutil.LoadAllTestsOrPanic(t, "testdata/k8s-ref") for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { // sanity check: input defines components assert.True(t, len(tt.Input.Workspace.Components) > 0, "Test case defines workspace with no components") - testClient := &fakeK8sClient{ - plugins: tt.Input.Plugins, - errors: tt.Input.Errors, + testClient := &testutil.FakeK8sClient{ + Plugins: tt.Input.Plugins, + Errors: tt.Input.Errors, } testResolverTools := ResolverTools{ Context: context.Background(), @@ -181,22 +43,22 @@ func TestResolveDevWorkspaceKubernetesReference(t *testing.T) { if !assert.NoError(t, err, "Should not return error") { return } - assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, workspaceTemplateDiffOpts), + assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts), "Workspace should match expected output:\n%s", - cmp.Diff(tt.Output.Workspace, outputWorkspace, workspaceTemplateDiffOpts)) + cmp.Diff(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts)) } }) } } func TestResolveDevWorkspaceInternalRegistry(t *testing.T) { - tests := loadAllTestsOrPanic(t, "testdata/internal-registry") - setupControllerCfg() + tests := testutil.LoadAllTestsOrPanic(t, "testdata/internal-registry") + testutil.SetupControllerCfg() for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { // sanity check: input defines components assert.True(t, len(tt.Input.Workspace.Components) > 0, "Test case defines workspace with no components") - testRegistry := &fakeInternalRegistry{ + testRegistry := &testutil.FakeInternalRegistry{ Plugins: tt.Input.Plugins, Errors: tt.Input.Errors, } @@ -211,9 +73,9 @@ func TestResolveDevWorkspaceInternalRegistry(t *testing.T) { if !assert.NoError(t, err, "Should not return error") { return } - assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, workspaceTemplateDiffOpts), + assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts), "Workspace should match expected output:\n%s", - cmp.Diff(tt.Output.Workspace, outputWorkspace, workspaceTemplateDiffOpts)) + cmp.Diff(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts)) } }) } diff --git a/pkg/library/flatten/testutil/common.go b/pkg/library/flatten/testutil/common.go new file mode 100644 index 000000000..74e0b4d50 --- /dev/null +++ b/pkg/library/flatten/testutil/common.go @@ -0,0 +1,106 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package testutil + +import ( + "fmt" + "io/ioutil" + "path/filepath" + "strings" + "testing" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/yaml" +) + +var WorkspaceTemplateDiffOpts = cmp.Options{ + cmpopts.SortSlices(func(a, b dw.Component) bool { + return strings.Compare(a.Key(), b.Key()) > 0 + }), + cmpopts.SortSlices(func(a, b string) bool { + return strings.Compare(a, b) > 0 + }), + // TODO: Devworkspace overriding results in empty []string instead of nil + cmpopts.IgnoreFields(dw.WorkspaceEvents{}, "PostStart", "PreStop", "PostStop"), +} + +var testControllerCfg = &corev1.ConfigMap{ + Data: map[string]string{ + "devworkspace.default_dockerimage.redhat-developer.web-terminal": ` +name: default-web-terminal-tooling +container: + name: default-web-terminal-tooling-container + image: test-image +`, + }, +} + +func SetupControllerCfg() { + config.SetupConfigForTesting(testControllerCfg) +} + +type TestCase struct { + Name string `json:"name"` + Input TestInput `json:"input"` + Output TestOutput `json:"output"` +} + +type TestInput struct { + Workspace dw.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` + // Plugins is a map of plugin "name" to devworkspace template; namespace is ignored. + Plugins map[string]dw.DevWorkspaceTemplate `json:"plugins,omitempty"` + // Errors is a map of plugin name to the error that should be returned when attempting to retrieve it. + Errors map[string]TestPluginError `json:"errors,omitempty"` +} + +type TestPluginError struct { + IsNotFound bool `json:"isNotFound"` + Message string `json:"message"` +} + +type TestOutput struct { + Workspace *dw.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` + ErrRegexp *string `json:"errRegexp,omitempty"` +} + +func LoadTestCaseOrPanic(t *testing.T, testFilepath string) TestCase { + bytes, err := ioutil.ReadFile(testFilepath) + if err != nil { + t.Fatal(err) + } + var test TestCase + if err := yaml.Unmarshal(bytes, &test); err != nil { + t.Fatal(err) + } + t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) + return test +} + +func LoadAllTestsOrPanic(t *testing.T, fromDir string) []TestCase { + files, err := ioutil.ReadDir(fromDir) + if err != nil { + t.Fatal(err) + } + var tests []TestCase + for _, file := range files { + if file.IsDir() { + continue + } + tests = append(tests, LoadTestCaseOrPanic(t, filepath.Join(fromDir, file.Name()))) + } + return tests +} diff --git a/pkg/library/flatten/testutil/internalRegistry.go b/pkg/library/flatten/testutil/internalRegistry.go new file mode 100644 index 000000000..b7d868f7b --- /dev/null +++ b/pkg/library/flatten/testutil/internalRegistry.go @@ -0,0 +1,41 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package testutil + +import ( + "errors" + "fmt" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" +) + +type FakeInternalRegistry struct { + Plugins map[string]dw.DevWorkspaceTemplate + Errors map[string]TestPluginError +} + +func (reg *FakeInternalRegistry) IsInInternalRegistry(pluginID string) bool { + _, pluginOk := reg.Plugins[pluginID] + _, errOk := reg.Errors[pluginID] + return pluginOk || errOk +} + +func (reg *FakeInternalRegistry) ReadPluginFromInternalRegistry(pluginID string) (*dw.DevWorkspaceTemplate, error) { + if plugin, ok := reg.Plugins[pluginID]; ok { + return &plugin, nil + } + if err, ok := reg.Errors[pluginID]; ok { + return nil, errors.New(err.Message) + } + return nil, fmt.Errorf("test does not define entry for plugin %s", pluginID) +} diff --git a/pkg/library/flatten/testutil/k8sClient.go b/pkg/library/flatten/testutil/k8sClient.go new file mode 100644 index 000000000..1694c7551 --- /dev/null +++ b/pkg/library/flatten/testutil/k8sClient.go @@ -0,0 +1,50 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package testutil + +import ( + "context" + "errors" + "fmt" + + "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type FakeK8sClient struct { + client.Client // To satisfy interface; override all used methods + Plugins map[string]v1alpha2.DevWorkspaceTemplate + Errors map[string]TestPluginError +} + +func (client *FakeK8sClient) Get(_ context.Context, namespacedName client.ObjectKey, obj runtime.Object) error { + template, ok := obj.(*v1alpha2.DevWorkspaceTemplate) + if !ok { + return fmt.Errorf("called Get() in fake client with non-DevWorkspaceTemplate") + } + if plugin, ok := client.Plugins[namespacedName.Name]; ok { + *template = plugin + return nil + } + if err, ok := client.Errors[namespacedName.Name]; ok { + if err.IsNotFound { + return k8sErrors.NewNotFound(schema.GroupResource{}, namespacedName.Name) + } else { + return errors.New(err.Message) + } + } + return fmt.Errorf("test does not define an entry for plugin %s", namespacedName.Name) +} From 1993472f8ee16d4830ff1746f600a3d10678d4c2 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 16 Feb 2021 17:00:32 -0500 Subject: [PATCH 3/7] Add HTTPGetter interface to allow mocking in tests Signed-off-by: Angel Misevski --- pkg/library/flatten/flatten.go | 12 ++++-------- pkg/library/flatten/network/fetch.go | 6 +++++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index b682142c3..83817efdc 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -15,18 +15,14 @@ package flatten import ( "context" "fmt" - "net/http" "net/url" "path" - "github.com/devfile/devworkspace-operator/pkg/library/flatten/network" - - "github.com/devfile/devworkspace-operator/pkg/library/flatten/web_terminal" - - registry "github.com/devfile/devworkspace-operator/pkg/library/flatten/internal_registry" - devworkspace "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/api/v2/pkg/utils/overriding" + registry "github.com/devfile/devworkspace-operator/pkg/library/flatten/internal_registry" + "github.com/devfile/devworkspace-operator/pkg/library/flatten/network" + "github.com/devfile/devworkspace-operator/pkg/library/flatten/web_terminal" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,7 +33,7 @@ type ResolverTools struct { Context context.Context K8sClient client.Client InternalRegistry registry.InternalRegistry - HttpClient *http.Client + HttpClient network.HTTPGetter } // ResolveDevWorkspace takes a devworkspace and returns a "resolved" version of it -- i.e. one where all plugins and parents diff --git a/pkg/library/flatten/network/fetch.go b/pkg/library/flatten/network/fetch.go index f738f82c0..2e1fc0b91 100644 --- a/pkg/library/flatten/network/fetch.go +++ b/pkg/library/flatten/network/fetch.go @@ -21,7 +21,11 @@ import ( "sigs.k8s.io/yaml" ) -func FetchDevWorkspaceTemplate(location string, httpClient *http.Client) (*dw.DevWorkspaceTemplateSpec, map[string]string, error) { +type HTTPGetter interface { + Get(location string) (*http.Response, error) +} + +func FetchDevWorkspaceTemplate(location string, httpClient HTTPGetter) (*dw.DevWorkspaceTemplateSpec, map[string]string, error) { resp, err := httpClient.Get(location) if err != nil { return nil, nil, fmt.Errorf("failed to fetch file from %s: %w", location, err) From b65b3273441cb26e3aac7aadbb121db8bedef521 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 16 Feb 2021 19:52:17 -0500 Subject: [PATCH 4/7] Add test cases to cover downloading plugins from URI Signed-off-by: Angel Misevski --- pkg/library/flatten/flatten_test.go | 60 ++++++++++++++++++ .../error_invalid-schema-version.yaml | 22 +++++++ .../testdata/plugin-id/error_on-fetch.yaml | 15 +++++ .../plugin-id/error_plugin-not-found.yaml | 15 +++++ .../plugin-id/error_unparseable-url.yaml | 12 ++++ .../plugin-id/resolve-plugin-by-id.yaml | 27 ++++++++ .../resolve-plugin-multiple-registries.yaml | 44 +++++++++++++ .../error_invalid-schema-version.yaml | 21 +++++++ .../testdata/plugin-uri/error_on-fetch.yaml | 14 +++++ .../plugin-uri/error_plugin-not-found.yaml | 14 +++++ .../resolve-multiple-plugins-by-uri.yaml | 42 +++++++++++++ .../plugin-uri/resolve-plugin-by-uri.yaml | 26 ++++++++ pkg/library/flatten/testutil/common.go | 13 ++-- pkg/library/flatten/testutil/http.go | 62 +++++++++++++++++++ 14 files changed, 383 insertions(+), 4 deletions(-) create mode 100644 pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml create mode 100644 pkg/library/flatten/testdata/plugin-id/error_on-fetch.yaml create mode 100644 pkg/library/flatten/testdata/plugin-id/error_plugin-not-found.yaml create mode 100644 pkg/library/flatten/testdata/plugin-id/error_unparseable-url.yaml create mode 100644 pkg/library/flatten/testdata/plugin-id/resolve-plugin-by-id.yaml create mode 100644 pkg/library/flatten/testdata/plugin-id/resolve-plugin-multiple-registries.yaml create mode 100644 pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml create mode 100644 pkg/library/flatten/testdata/plugin-uri/error_on-fetch.yaml create mode 100644 pkg/library/flatten/testdata/plugin-uri/error_plugin-not-found.yaml create mode 100644 pkg/library/flatten/testdata/plugin-uri/resolve-multiple-plugins-by-uri.yaml create mode 100644 pkg/library/flatten/testdata/plugin-uri/resolve-plugin-by-uri.yaml create mode 100644 pkg/library/flatten/testutil/http.go diff --git a/pkg/library/flatten/flatten_test.go b/pkg/library/flatten/flatten_test.go index 2a82e639f..2db3e3a24 100644 --- a/pkg/library/flatten/flatten_test.go +++ b/pkg/library/flatten/flatten_test.go @@ -80,3 +80,63 @@ func TestResolveDevWorkspaceInternalRegistry(t *testing.T) { }) } } + +func TestResolveDevWorkspacePluginRegistry(t *testing.T) { + tests := testutil.LoadAllTestsOrPanic(t, "testdata/plugin-id") + testutil.SetupControllerCfg() + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check: input defines components + assert.True(t, len(tt.Input.Workspace.Components) > 0, "Test case defines workspace with no components") + testHttpGetter := &testutil.FakeHTTPGetter{ + Plugins: tt.Input.DevfilePlugins, + Errors: tt.Input.Errors, + } + testResolverTools := ResolverTools{ + Context: context.Background(), + HttpClient: testHttpGetter, + } + outputWorkspace, err := ResolveDevWorkspace(tt.Input.Workspace, testResolverTools) + if tt.Output.ErrRegexp != nil && assert.Error(t, err) { + assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") + } else { + if !assert.NoError(t, err, "Should not return error") { + return + } + assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts), + "Workspace should match expected output:\n%s", + cmp.Diff(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts)) + } + }) + } +} + +func TestResolveDevWorkspacePluginURI(t *testing.T) { + tests := testutil.LoadAllTestsOrPanic(t, "testdata/plugin-uri") + testutil.SetupControllerCfg() + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + // sanity check: input defines components + assert.True(t, len(tt.Input.Workspace.Components) > 0, "Test case defines workspace with no components") + testHttpGetter := &testutil.FakeHTTPGetter{ + Plugins: tt.Input.DevfilePlugins, + Errors: tt.Input.Errors, + } + testResolverTools := ResolverTools{ + Context: context.Background(), + HttpClient: testHttpGetter, + } + outputWorkspace, err := ResolveDevWorkspace(tt.Input.Workspace, testResolverTools) + if tt.Output.ErrRegexp != nil && assert.Error(t, err) { + assert.Regexp(t, *tt.Output.ErrRegexp, err.Error(), "Error message should match") + } else { + if !assert.NoError(t, err, "Should not return error") { + return + } + assert.Truef(t, cmp.Equal(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts), + "Workspace should match expected output:\n%s", + cmp.Diff(tt.Output.Workspace, outputWorkspace, testutil.WorkspaceTemplateDiffOpts)) + } + }) + } +} diff --git a/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml b/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml new file mode 100644 index 000000000..0cf32d8c2 --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml @@ -0,0 +1,22 @@ +name: "DevWorkspace references plugin with invalid schemaVersion" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: "https://test-registry.io/subpath" + devfilePlugins: + "https://test-registry.io/subpath/plugins/my/test/plugin": + schemaVersion: 1.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + +output: + errRegexp: "could not process devfile: supported schemaVersion is .*" diff --git a/pkg/library/flatten/testdata/plugin-id/error_on-fetch.yaml b/pkg/library/flatten/testdata/plugin-id/error_on-fetch.yaml new file mode 100644 index 000000000..92a44a691 --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/error_on-fetch.yaml @@ -0,0 +1,15 @@ +name: "Error when fetching plugin" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: "https://test-registry.io/subpath" + errors: + "https://test-registry.io/subpath/plugins/my/test/plugin": + message: "testing error" + +output: + errRegexp: "failed to fetch file from.*testing error" diff --git a/pkg/library/flatten/testdata/plugin-id/error_plugin-not-found.yaml b/pkg/library/flatten/testdata/plugin-id/error_plugin-not-found.yaml new file mode 100644 index 000000000..36da9f474 --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/error_plugin-not-found.yaml @@ -0,0 +1,15 @@ +name: "Plugin not found in registry" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: "https://test-registry.io/subpath" + errors: + "https://test-registry.io/subpath/plugins/my/test/plugin": + statusCode: 404 + +output: + errRegexp: "could not fetch file from.*got status 404" diff --git a/pkg/library/flatten/testdata/plugin-id/error_unparseable-url.yaml b/pkg/library/flatten/testdata/plugin-id/error_unparseable-url.yaml new file mode 100644 index 000000000..62fd534ac --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/error_unparseable-url.yaml @@ -0,0 +1,12 @@ +name: "Error when parsing registryURL" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: ":/test-registry.io/subpath" + +output: + errRegexp: "failed to parse registry URL for plugin test-plugin" diff --git a/pkg/library/flatten/testdata/plugin-id/resolve-plugin-by-id.yaml b/pkg/library/flatten/testdata/plugin-id/resolve-plugin-by-id.yaml new file mode 100644 index 000000000..d5edf2c5c --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/resolve-plugin-by-id.yaml @@ -0,0 +1,27 @@ +name: "DevWorkspace references plugin from plugin registry" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: "https://test-registry.io/subpath" + devfilePlugins: + "https://test-registry.io/subpath/plugins/my/test/plugin": + schemaVersion: 2.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + +output: + workspace: + components: + - name: plugin-a + container: + name: test-container + image: test-image diff --git a/pkg/library/flatten/testdata/plugin-id/resolve-plugin-multiple-registries.yaml b/pkg/library/flatten/testdata/plugin-id/resolve-plugin-multiple-registries.yaml new file mode 100644 index 000000000..17faa7f6e --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-id/resolve-plugin-multiple-registries.yaml @@ -0,0 +1,44 @@ +name: "DevWorkspace references plugins from multiple plugin registries" + +input: + workspace: + components: + - name: test-plugin + plugin: + id: my/test/plugin + registryUrl: "https://test-registry.io/subpath" + - name: test-plugin-2 + plugin: + id: my/test/plugin-2 + registryUrl: "https://test-registry-2.io/subpath" + devfilePlugins: + "https://test-registry.io/subpath/plugins/my/test/plugin": + schemaVersion: 2.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + "https://test-registry-2.io/subpath/plugins/my/test/plugin-2": + schemaVersion: 2.0.0 + metadata: + name: "plugin-b" + components: + - name: plugin-b + container: + name: test-container-b + image: test-image + +output: + workspace: + components: + - name: plugin-a + container: + name: test-container + image: test-image + - name: plugin-b + container: + name: test-container-b + image: test-image diff --git a/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml b/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml new file mode 100644 index 000000000..8980217cc --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml @@ -0,0 +1,21 @@ +name: "DevWorkspace references plugin with invalid schemaVersion" + +input: + workspace: + components: + - name: test-plugin + plugin: + uri: https://test-registry.io/old-devfiles + devfilePlugins: + "https://test-registry.io/old-devfiles": + schemaVersion: 1.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + +output: + errRegexp: "could not process devfile: supported schemaVersion is .*" diff --git a/pkg/library/flatten/testdata/plugin-uri/error_on-fetch.yaml b/pkg/library/flatten/testdata/plugin-uri/error_on-fetch.yaml new file mode 100644 index 000000000..45bfd6d75 --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-uri/error_on-fetch.yaml @@ -0,0 +1,14 @@ +name: "Error when fetching plugin" + +input: + workspace: + components: + - name: test-plugin + plugin: + uri: https://test-registry.io/error + errors: + "https://test-registry.io/error": + message: "testing error" + +output: + errRegexp: "failed to fetch file from.*testing error" diff --git a/pkg/library/flatten/testdata/plugin-uri/error_plugin-not-found.yaml b/pkg/library/flatten/testdata/plugin-uri/error_plugin-not-found.yaml new file mode 100644 index 000000000..5a1da1af5 --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-uri/error_plugin-not-found.yaml @@ -0,0 +1,14 @@ +name: "Plugin not found in at URI" + +input: + workspace: + components: + - name: test-plugin + plugin: + uri: "https://test-registry.io/notfound" + errors: + "https://test-registry.io/notfound": + statusCode: 404 + +output: + errRegexp: "could not fetch file from.*got status 404" diff --git a/pkg/library/flatten/testdata/plugin-uri/resolve-multiple-plugins-by-uri.yaml b/pkg/library/flatten/testdata/plugin-uri/resolve-multiple-plugins-by-uri.yaml new file mode 100644 index 000000000..4d39474bb --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-uri/resolve-multiple-plugins-by-uri.yaml @@ -0,0 +1,42 @@ +name: "DevWorkspace references plugins from multiple plugin registries" + +input: + workspace: + components: + - name: test-plugin + plugin: + uri: "https://my-plugin.io/test" + - name: test-plugin-2 + plugin: + uri: "https://my-plugin-alt.io/test" + devfilePlugins: + "https://my-plugin.io/test": + schemaVersion: 2.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + "https://my-plugin-alt.io/test": + schemaVersion: 2.0.0 + metadata: + name: "plugin-b" + components: + - name: plugin-b + container: + name: test-container-b + image: test-image + +output: + workspace: + components: + - name: plugin-a + container: + name: test-container + image: test-image + - name: plugin-b + container: + name: test-container-b + image: test-image diff --git a/pkg/library/flatten/testdata/plugin-uri/resolve-plugin-by-uri.yaml b/pkg/library/flatten/testdata/plugin-uri/resolve-plugin-by-uri.yaml new file mode 100644 index 000000000..cb37e0bef --- /dev/null +++ b/pkg/library/flatten/testdata/plugin-uri/resolve-plugin-by-uri.yaml @@ -0,0 +1,26 @@ +name: "DevWorkspace references plugin by URI" + +input: + workspace: + components: + - name: test-plugin + plugin: + uri: "https://my-plugin.io/test" + devfilePlugins: + "https://my-plugin.io/test": + schemaVersion: 2.0.0 + metadata: + name: "plugin-a" + components: + - name: plugin-a + container: + name: test-container + image: test-image + +output: + workspace: + components: + - name: plugin-a + container: + name: test-container + image: test-image diff --git a/pkg/library/flatten/testutil/common.go b/pkg/library/flatten/testutil/common.go index 74e0b4d50..19ad7084f 100644 --- a/pkg/library/flatten/testutil/common.go +++ b/pkg/library/flatten/testutil/common.go @@ -13,7 +13,6 @@ package testutil import ( - "fmt" "io/ioutil" "path/filepath" "strings" @@ -21,6 +20,7 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/library/flatten/network" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" corev1 "k8s.io/api/core/v1" @@ -63,13 +63,19 @@ type TestInput struct { Workspace dw.DevWorkspaceTemplateSpec `json:"workspace,omitempty"` // Plugins is a map of plugin "name" to devworkspace template; namespace is ignored. Plugins map[string]dw.DevWorkspaceTemplate `json:"plugins,omitempty"` + // DevfilePlugins is a map of plugin "name" to devfile + DevfilePlugins map[string]network.Devfile `json:"devfilePlugins,omitempty"` // Errors is a map of plugin name to the error that should be returned when attempting to retrieve it. Errors map[string]TestPluginError `json:"errors,omitempty"` } type TestPluginError struct { - IsNotFound bool `json:"isNotFound"` - Message string `json:"message"` + // IsNotFound marks this error as a kubernetes NotFoundError + IsNotFound bool `json:"isNotFound"` + // StatusCode defines the HTTP response code (if relevant) + StatusCode int `json:"statusCode"` + // Message is the error message returned + Message string `json:"message"` } type TestOutput struct { @@ -86,7 +92,6 @@ func LoadTestCaseOrPanic(t *testing.T, testFilepath string) TestCase { if err := yaml.Unmarshal(bytes, &test); err != nil { t.Fatal(err) } - t.Log(fmt.Sprintf("Read file:\n%+v\n\n", test)) return test } diff --git a/pkg/library/flatten/testutil/http.go b/pkg/library/flatten/testutil/http.go new file mode 100644 index 000000000..e0b2dfefb --- /dev/null +++ b/pkg/library/flatten/testutil/http.go @@ -0,0 +1,62 @@ +// +// Copyright (c) 2019-2021 Red Hat, Inc. +// This program and the accompanying materials are made +// available under the terms of the Eclipse Public License 2.0 +// which is available at https://www.eclipse.org/legal/epl-2.0/ +// +// SPDX-License-Identifier: EPL-2.0 +// +// Contributors: +// Red Hat, Inc. - initial API and implementation +// + +package testutil + +import ( + "bytes" + "errors" + "fmt" + "io" + "net/http" + + "sigs.k8s.io/yaml" + + "github.com/devfile/devworkspace-operator/pkg/library/flatten/network" +) + +type FakeHTTPGetter struct { + Plugins map[string]network.Devfile + Errors map[string]TestPluginError +} + +var _ network.HTTPGetter = (*FakeHTTPGetter)(nil) + +type fakeRespBody struct { + io.Reader +} + +func (_ *fakeRespBody) Close() error { return nil } + +func (reg *FakeHTTPGetter) Get(location string) (*http.Response, error) { + if plugin, ok := reg.Plugins[location]; ok { + yamlBytes, err := yaml.Marshal(plugin) + if err != nil { + return nil, fmt.Errorf("error marshalling plugin in test: %w", err) + } + resp := &http.Response{ + StatusCode: http.StatusOK, + Body: &fakeRespBody{bytes.NewBuffer(yamlBytes)}, + } + return resp, nil + } + if err, ok := reg.Errors[location]; ok { + if err.StatusCode != 0 { + return &http.Response{ + StatusCode: err.StatusCode, + Body: &fakeRespBody{bytes.NewBuffer([]byte{})}, + }, nil + } + return nil, errors.New(err.Message) + } + return nil, fmt.Errorf("test does not define entry for plugin at URL %s", location) +} From 8e46c078ae449a4269830d85d2226da59a3bda07 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 16 Feb 2021 19:55:56 -0500 Subject: [PATCH 5/7] Move testutils to internal package to avoid exporting Signed-off-by: Angel Misevski --- pkg/library/flatten/flatten_test.go | 2 +- pkg/library/flatten/{ => internal}/testutil/common.go | 0 pkg/library/flatten/{ => internal}/testutil/http.go | 0 pkg/library/flatten/{ => internal}/testutil/internalRegistry.go | 0 pkg/library/flatten/{ => internal}/testutil/k8sClient.go | 0 5 files changed, 1 insertion(+), 1 deletion(-) rename pkg/library/flatten/{ => internal}/testutil/common.go (100%) rename pkg/library/flatten/{ => internal}/testutil/http.go (100%) rename pkg/library/flatten/{ => internal}/testutil/internalRegistry.go (100%) rename pkg/library/flatten/{ => internal}/testutil/k8sClient.go (100%) diff --git a/pkg/library/flatten/flatten_test.go b/pkg/library/flatten/flatten_test.go index 2db3e3a24..64ad8b371 100644 --- a/pkg/library/flatten/flatten_test.go +++ b/pkg/library/flatten/flatten_test.go @@ -16,7 +16,7 @@ import ( "context" "testing" - "github.com/devfile/devworkspace-operator/pkg/library/flatten/testutil" + "github.com/devfile/devworkspace-operator/pkg/library/flatten/internal/testutil" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" diff --git a/pkg/library/flatten/testutil/common.go b/pkg/library/flatten/internal/testutil/common.go similarity index 100% rename from pkg/library/flatten/testutil/common.go rename to pkg/library/flatten/internal/testutil/common.go diff --git a/pkg/library/flatten/testutil/http.go b/pkg/library/flatten/internal/testutil/http.go similarity index 100% rename from pkg/library/flatten/testutil/http.go rename to pkg/library/flatten/internal/testutil/http.go diff --git a/pkg/library/flatten/testutil/internalRegistry.go b/pkg/library/flatten/internal/testutil/internalRegistry.go similarity index 100% rename from pkg/library/flatten/testutil/internalRegistry.go rename to pkg/library/flatten/internal/testutil/internalRegistry.go diff --git a/pkg/library/flatten/testutil/k8sClient.go b/pkg/library/flatten/internal/testutil/k8sClient.go similarity index 100% rename from pkg/library/flatten/testutil/k8sClient.go rename to pkg/library/flatten/internal/testutil/k8sClient.go From a103cabf65cded89503517848a08fe17a12a34cc Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 19 Feb 2021 11:49:47 -0500 Subject: [PATCH 6/7] Check devfile schemaVersion via regexp to support all 2.x devfiles Signed-off-by: Angel Misevski --- pkg/library/flatten/network/devfile.go | 9 ++++----- .../testdata/plugin-id/error_invalid-schema-version.yaml | 2 +- .../plugin-uri/error_invalid-schema-version.yaml | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/library/flatten/network/devfile.go b/pkg/library/flatten/network/devfile.go index 84392b8a0..044eb5051 100644 --- a/pkg/library/flatten/network/devfile.go +++ b/pkg/library/flatten/network/devfile.go @@ -14,14 +14,13 @@ package network import ( "fmt" + "regexp" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" devfilev2 "github.com/devfile/api/v2/pkg/devfile" ) -const ( - SupportedDevfileSchemaVersion = "2.0.0" // TODO what should this actually be at this point? -) +var SupportedSchemaVersionRegexp = regexp.MustCompile(`^2\..+`) type Devfile struct { devfilev2.DevfileHeader @@ -29,8 +28,8 @@ type Devfile struct { } func ConvertDevfileToDevWorkspaceTemplate(devfile *Devfile) (*dw.DevWorkspaceTemplate, error) { - if devfile.SchemaVersion != SupportedDevfileSchemaVersion { - return nil, fmt.Errorf("could not process devfile: supported schemaVersion is %s", SupportedDevfileSchemaVersion) + if !SupportedSchemaVersionRegexp.MatchString(devfile.SchemaVersion) { + return nil, fmt.Errorf("could not process devfile: unsupported schemaVersion '%s'", devfile.SchemaVersion) } dwt := &dw.DevWorkspaceTemplate{} dwt.Spec = devfile.DevWorkspaceTemplateSpec diff --git a/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml b/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml index 0cf32d8c2..9b2839acc 100644 --- a/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml +++ b/pkg/library/flatten/testdata/plugin-id/error_invalid-schema-version.yaml @@ -19,4 +19,4 @@ input: image: test-image output: - errRegexp: "could not process devfile: supported schemaVersion is .*" + errRegexp: "could not process devfile: unsupported schemaVersion '1.0.0'" diff --git a/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml b/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml index 8980217cc..ea5f777a8 100644 --- a/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml +++ b/pkg/library/flatten/testdata/plugin-uri/error_invalid-schema-version.yaml @@ -18,4 +18,4 @@ input: image: test-image output: - errRegexp: "could not process devfile: supported schemaVersion is .*" + errRegexp: "could not process devfile: unsupported schemaVersion '1.0.0'" From 69aa54c9099a77fb8bca519aa03a4a9c0b934a37 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 19 Feb 2021 11:55:38 -0500 Subject: [PATCH 7/7] Removing handling of plugin metadata Avoid processing plugin metadata as there's still not defined way to translate metadata from a devfile into a DevWorkspace[Template] Signed-off-by: Angel Misevski --- pkg/library/flatten/flatten.go | 51 ++++++++++++++-------------- pkg/library/flatten/helper.go | 1 - pkg/library/flatten/network/fetch.go | 14 ++++---- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/pkg/library/flatten/flatten.go b/pkg/library/flatten/flatten.go index 83817efdc..72a02ce5a 100644 --- a/pkg/library/flatten/flatten.go +++ b/pkg/library/flatten/flatten.go @@ -76,12 +76,11 @@ func recursiveResolve(workspace devworkspace.DevWorkspaceTemplateSpec, tooling R // No action necessary resolvedContent.Components = append(resolvedContent.Components, component) } else { - pluginComponent, pluginMeta, err := resolvePluginComponent(component.Name, component.Plugin, tooling) + pluginComponent, err := resolvePluginComponent(component.Name, component.Plugin, tooling) if err != nil { return nil, err } newCtx := resolveCtx.addPlugin(component.Name, component.Plugin) - newCtx.pluginMetadata = pluginMeta if err := newCtx.hasCycle(); err != nil { return nil, err } @@ -107,7 +106,7 @@ func recursiveResolve(workspace devworkspace.DevWorkspaceTemplateSpec, tooling R func resolvePluginComponent( name string, plugin *devworkspace.PluginComponent, - tooling ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, pluginMeta map[string]string, err error) { + tooling ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, err error) { switch { // TODO: Add support for plugin ID and URI case plugin.Kubernetes != nil: @@ -115,16 +114,16 @@ func resolvePluginComponent( if plugin.Kubernetes.Namespace == "" { plugin.Kubernetes.Namespace = tooling.InstanceNamespace } - resolvedPlugin, pluginMeta, err = resolvePluginComponentByKubernetesReference(name, plugin, tooling) + resolvedPlugin, err = resolvePluginComponentByKubernetesReference(name, plugin, tooling) case plugin.Uri != "": - resolvedPlugin, pluginMeta, err = resolvePluginComponentByURI(name, plugin, tooling) + resolvedPlugin, err = resolvePluginComponentByURI(name, plugin, tooling) case plugin.Id != "": - resolvedPlugin, pluginMeta, err = resolvePluginComponentById(name, plugin, tooling) + resolvedPlugin, err = resolvePluginComponentById(name, plugin, tooling) default: err = fmt.Errorf("plugin %s does not define any resources", name) } if err != nil { - return nil, nil, err + return nil, err } if plugin.Components != nil || plugin.Commands != nil { @@ -134,17 +133,17 @@ func resolvePluginComponent( }) if err != nil { - return nil, nil, err + return nil, err } resolvedPlugin.DevWorkspaceTemplateSpecContent = *overrideSpec } - return resolvedPlugin, pluginMeta, nil + return resolvedPlugin, nil } func resolvePluginComponentByKubernetesReference( name string, plugin *devworkspace.PluginComponent, - tooling ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, pluginLabels map[string]string, err error) { + tooling ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, err error) { var dwTemplate devworkspace.DevWorkspaceTemplate namespacedName := types.NamespacedName{ @@ -154,54 +153,54 @@ func resolvePluginComponentByKubernetesReference( err = tooling.K8sClient.Get(tooling.Context, namespacedName, &dwTemplate) if err != nil { if errors.IsNotFound(err) { - return nil, nil, fmt.Errorf("plugin for component %s not found", name) + return nil, fmt.Errorf("plugin for component %s not found", name) } - return nil, nil, fmt.Errorf("failed to retrieve plugin referenced by kubernetes name and namespace '%s': %w", name, err) + return nil, fmt.Errorf("failed to retrieve plugin referenced by kubernetes name and namespace '%s': %w", name, err) } - return &dwTemplate.Spec, dwTemplate.Labels, nil + return &dwTemplate.Spec, nil } func resolvePluginComponentById( name string, plugin *devworkspace.PluginComponent, - tools ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, pluginLabels map[string]string, err error) { + tools ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, err error) { // Check internal registry for plugins that do not specify a registry if plugin.RegistryUrl == "" { if tools.InternalRegistry == nil { - return nil, nil, fmt.Errorf("plugin %s does not specify a registryUrl and no internal registry is configured", name) + return nil, fmt.Errorf("plugin %s does not specify a registryUrl and no internal registry is configured", name) } if !tools.InternalRegistry.IsInInternalRegistry(plugin.Id) { - return nil, nil, fmt.Errorf("plugin for component %s does not specify a registry and is not present in the internal registry", name) + return nil, fmt.Errorf("plugin for component %s does not specify a registry and is not present in the internal registry", name) } pluginDWT, err := tools.InternalRegistry.ReadPluginFromInternalRegistry(plugin.Id) if err != nil { - return nil, nil, fmt.Errorf("failed to read plugin for component %s from internal registry: %w", name, err) + return nil, fmt.Errorf("failed to read plugin for component %s from internal registry: %w", name, err) } - return &pluginDWT.Spec, pluginDWT.Labels, nil + return &pluginDWT.Spec, nil } pluginURL, err := url.Parse(plugin.RegistryUrl) if err != nil { - return nil, nil, fmt.Errorf("failed to parse registry URL for plugin %s: %w", name, err) + return nil, fmt.Errorf("failed to parse registry URL for plugin %s: %w", name, err) } pluginURL.Path = path.Join(pluginURL.Path, "plugins", plugin.Id) - dwt, labels, err := network.FetchDevWorkspaceTemplate(pluginURL.String(), tools.HttpClient) + dwt, err := network.FetchDevWorkspaceTemplate(pluginURL.String(), tools.HttpClient) if err != nil { - return nil, nil, fmt.Errorf("failed to resolve plugin %s from registry %s: %w", name, plugin.RegistryUrl, err) + return nil, fmt.Errorf("failed to resolve plugin %s from registry %s: %w", name, plugin.RegistryUrl, err) } - return dwt, labels, nil + return dwt, nil } func resolvePluginComponentByURI( name string, plugin *devworkspace.PluginComponent, - tools ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, pluginLabels map[string]string, err error) { + tools ResolverTools) (resolvedPlugin *devworkspace.DevWorkspaceTemplateSpec, err error) { - dwt, labels, err := network.FetchDevWorkspaceTemplate(plugin.Uri, tools.HttpClient) + dwt, err := network.FetchDevWorkspaceTemplate(plugin.Uri, tools.HttpClient) if err != nil { - return nil, nil, fmt.Errorf("failed to resolve plugin %s by URI: %w", name, err) + return nil, fmt.Errorf("failed to resolve plugin %s by URI: %w", name, err) } - return dwt, labels, nil + return dwt, nil } diff --git a/pkg/library/flatten/helper.go b/pkg/library/flatten/helper.go index b26b89ca4..8d9ab02dc 100644 --- a/pkg/library/flatten/helper.go +++ b/pkg/library/flatten/helper.go @@ -24,7 +24,6 @@ import ( type resolutionContextTree struct { componentName string importReference devworkspace.ImportReference - pluginMetadata map[string]string plugins []*resolutionContextTree parentNode *resolutionContextTree } diff --git a/pkg/library/flatten/network/fetch.go b/pkg/library/flatten/network/fetch.go index 2e1fc0b91..f07d1344b 100644 --- a/pkg/library/flatten/network/fetch.go +++ b/pkg/library/flatten/network/fetch.go @@ -25,31 +25,31 @@ type HTTPGetter interface { Get(location string) (*http.Response, error) } -func FetchDevWorkspaceTemplate(location string, httpClient HTTPGetter) (*dw.DevWorkspaceTemplateSpec, map[string]string, error) { +func FetchDevWorkspaceTemplate(location string, httpClient HTTPGetter) (*dw.DevWorkspaceTemplateSpec, error) { resp, err := httpClient.Get(location) if err != nil { - return nil, nil, fmt.Errorf("failed to fetch file from %s: %w", location, err) + return nil, fmt.Errorf("failed to fetch file from %s: %w", location, err) } defer resp.Body.Close() // ignoring error because what would we even do? if resp.StatusCode != http.StatusOK { - return nil, nil, fmt.Errorf("could not fetch file from %s: got status %d", location, resp.StatusCode) + return nil, fmt.Errorf("could not fetch file from %s: got status %d", location, resp.StatusCode) } bytes, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, nil, fmt.Errorf("could not read data from %s: %w", location, err) + return nil, fmt.Errorf("could not read data from %s: %w", location, err) } // Assume we're getting a devfile, not a DevWorkspaceTemplate (TODO: Detect type and handle both?) devfile := &Devfile{} err = yaml.Unmarshal(bytes, devfile) if err != nil { - return nil, nil, fmt.Errorf("could not unmarshal devfile from response: %w", err) + return nil, fmt.Errorf("could not unmarshal devfile from response: %w", err) } dwt, err := ConvertDevfileToDevWorkspaceTemplate(devfile) if err != nil { - return nil, nil, fmt.Errorf("failed to convert devfile to DevWorkspaceTemplate: %s", err) + return nil, fmt.Errorf("failed to convert devfile to DevWorkspaceTemplate: %s", err) } - return &dwt.Spec, dwt.Labels, nil + return &dwt.Spec, nil }