Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for parsing multiple k8s definition in a single Devfile K8s component #6372

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/binding/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (o *BindingClient) GetBindingsFromDevfile(devfileObj parser.DevfileObj, con
}

for _, component := range kubeComponents {
strCRD, err := libdevfile.GetK8sManifestWithVariablesSubstituted(devfileObj, component.Name, context, devfilefs.DefaultFs{})
strCRD, err := libdevfile.GetK8sManifestsWithVariablesSubstituted(devfileObj, component.Name, context, devfilefs.DefaultFs{})
if err != nil {
return nil, err
}
Expand Down
9 changes: 6 additions & 3 deletions pkg/binding/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,15 @@ func (o *BindingClient) RemoveBinding(servicebindingName string, obj parser.Devf
return obj, err
}
for _, component := range components {
var unstructuredObj unstructured.Unstructured
var unstructuredObjs []unstructured.Unstructured
// Parse the K8s manifest
unstructuredObj, err = libdevfile.GetK8sComponentAsUnstructured(obj, component.Name, filepath.Dir(obj.Ctx.GetAbsPath()), devfilefs.DefaultFs{})
if err != nil {
unstructuredObjs, err = libdevfile.GetK8sComponentAsUnstructuredList(obj, component.Name, filepath.Dir(obj.Ctx.GetAbsPath()), devfilefs.DefaultFs{})
if err != nil || len(unstructuredObjs) == 0 {
continue
}
// We default to the first object in the list because as far as ServiceBinding is concerned,
// we assume that only one resource will be defined for the Devfile K8s component; which is true
unstructuredObj := unstructuredObjs[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that len(unstructuredObjs) > 0 before, or this will panic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's unlikely that the list will ever be empty. I've tried to run the remove binding command on non-existent bindings and it's never panicked. But you're right, I'll add the check to be on the safe side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One should never under-estimate the imagination of users ...

if unstructuredObj.GetKind() == kclient.ServiceBindingKind {
options = append(options, unstructuredObj.GetName())
if unstructuredObj.GetName() == servicebindingName {
Expand Down
24 changes: 13 additions & 11 deletions pkg/component/apply_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import (
devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/pkg/devfile/parser"
devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
"k8s.io/klog"

"github.com/redhat-developer/odo/pkg/kclient"
odolabels "github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/libdevfile"
"github.com/redhat-developer/odo/pkg/log"
"github.com/redhat-developer/odo/pkg/service"
"k8s.io/klog"
)

// ApplyKubernetes contains the logic to create the k8s resources defined by the `apply` command
Expand All @@ -30,10 +31,11 @@ func ApplyKubernetes(
kubeClient kclient.ClientInterface,
path string,
) error {
// TODO: Use GetK8sComponentAsUnstructured here and pass it to ValidateResourcesExistInK8sComponent
// Validate if the GVRs represented by Kubernetes inlined components are supported by the underlying cluster
_, err := ValidateResourceExist(kubeClient, devfile, kubernetes, path)
kind, err := ValidateResourcesExistInK8sComponent(kubeClient, devfile, kubernetes, path)
if err != nil {
return err
return fmt.Errorf("%s: %w", kind, err)
}

// Get the most common labels that's applicable to all resources being deployed.
Expand All @@ -49,17 +51,17 @@ func ApplyKubernetes(
odolabels.SetProjectType(annotations, GetComponentTypeFromDevfileMetadata(devfile.Data.GetMetadata()))

// Get the Kubernetes component
u, err := libdevfile.GetK8sComponentAsUnstructured(devfile, kubernetes.Name, path, devfilefs.DefaultFs{})
uList, err := libdevfile.GetK8sComponentAsUnstructuredList(devfile, kubernetes.Name, path, devfilefs.DefaultFs{})
if err != nil {
return err
}

// Deploy the actual Kubernetes component and error out if there's an issue.
log.Sectionf("Deploying Kubernetes Component: %s", u.GetName())
err = service.PushKubernetesResource(kubeClient, u, labels, annotations, mode)
if err != nil {
return fmt.Errorf("failed to create service(s) associated with the component: %w", err)
for _, u := range uList {
// Deploy the actual Kubernetes component and error out if there's an issue.
log.Sectionf("Deploying Kubernetes Component: %s", u.GetName())
err = service.PushKubernetesResource(kubeClient, u, labels, annotations, mode)
if err != nil {
return fmt.Errorf("failed to create service(s) associated with the component: %w", err)
}
}

return nil
}
8 changes: 6 additions & 2 deletions pkg/component/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,15 +506,19 @@ func TestListRoutesAndIngresses(t *testing.T) {
)
createFakeIngressFromDevfile := func(devfileObj parser.DevfileObj, ingressComponentName string, label map[string]string) *v1.Ingress {
ing := &v1.Ingress{}
u, _ := libdevfile.GetK8sComponentAsUnstructured(devfileObj, ingressComponentName, "", filesystem.DefaultFs{})
uList, _ := libdevfile.GetK8sComponentAsUnstructuredList(devfileObj, ingressComponentName, "", filesystem.DefaultFs{})
// We default to the first object in the list because it is safe to do so since we have only defined one K8s resource for the Devfile K8s component
u := uList[0]
_ = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), ing)
ing.SetLabels(label)
return ing
}

createFakeRouteFromDevfile := func(devfileObj parser.DevfileObj, routeComponentName string, label map[string]string) *v12.Route {
route := &v12.Route{}
u, _ := libdevfile.GetK8sComponentAsUnstructured(devfileObj, routeComponentName, "", filesystem.DefaultFs{})
uList, _ := libdevfile.GetK8sComponentAsUnstructuredList(devfileObj, routeComponentName, "", filesystem.DefaultFs{})
// We default to the first object in the list because it is safe to do so since we have only defined one K8s resource for the Devfile K8s component
u := uList[0]
_ = runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), route)
route.SetLabels(label)
return route
Expand Down
23 changes: 12 additions & 11 deletions pkg/component/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func ValidateResourcesExist(client kclient.ClientInterface, devfileObj parser.De

var unsupportedResources []string
for _, c := range k8sComponents {
kindErr, err := ValidateResourceExist(client, devfileObj, c, context)
kindErr, err := ValidateResourcesExistInK8sComponent(client, devfileObj, c, context)
if err != nil {
if kindErr != "" {
unsupportedResources = append(unsupportedResources, kindErr)
Expand All @@ -38,20 +38,21 @@ func ValidateResourcesExist(client kclient.ClientInterface, devfileObj parser.De
return nil
}

// ValidateResourceExist validates if a Kubernetes inlined component is installed on the cluster
func ValidateResourceExist(client kclient.ClientInterface, devfileObj parser.DevfileObj, k8sComponent devfile.Component, context string) (kindErr string, err error) {
// ValidateResourcesExistInK8sComponent validates if resources defined inside a Kubernetes inlined component are installed on the cluster
func ValidateResourcesExistInK8sComponent(client kclient.ClientInterface, devfileObj parser.DevfileObj, k8sComponent devfile.Component, context string) (kindErr string, err error) {
// get the string representation of the YAML definition of a CRD
u, err := libdevfile.GetK8sComponentAsUnstructured(devfileObj, k8sComponent.Name, context, devfilefs.DefaultFs{})
uList, err := libdevfile.GetK8sComponentAsUnstructuredList(devfileObj, k8sComponent.Name, context, devfilefs.DefaultFs{})
if err != nil {
return "", err
}

_, err = client.GetRestMappingFromUnstructured(u)
if err != nil && u.GetKind() != "ServiceBinding" {
// getting a RestMapping would fail if there are no matches for the Kind field on the cluster;
// but if it's a "ServiceBinding" resource, we don't add it to unsupported list because odo can create links
// without having SBO installed
return u.GetKind(), errors.New("resource not supported")
for _, u := range uList {
_, err = client.GetRestMappingFromUnstructured(u)
if err != nil && u.GetKind() != "ServiceBinding" {
// getting a RestMapping would fail if there are no matches for the Kind field on the cluster;
// but if it's a "ServiceBinding" resource, we don't add it to unsupported list because odo can create links
// without having SBO installed
return u.GetKind(), errors.New("resource not supported")
}
}
return "", nil
}
8 changes: 4 additions & 4 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ func (a *Adapter) createOrUpdateComponent(
}

func (a *Adapter) createOrUpdateServiceForComponent(svc *corev1.Service, componentName string, ownerReference metav1.OwnerReference) error {
oldSvc, err := a.kubeClient.GetOneService(a.ComponentName, a.AppName)
oldSvc, err := a.kubeClient.GetOneService(a.ComponentName, a.AppName, true)
originOwnerReferences := svc.OwnerReferences
if err != nil {
// no old service was found, create a new one
Expand Down Expand Up @@ -635,12 +635,12 @@ func (a Adapter) getRemoteResourcesNotPresentInDevfile(selector string) (objects
// convert all devfileK8sResources to unstructured data
var devfileK8sResourcesUnstructured []unstructured.Unstructured
for _, devfileK := range devfileK8sResources {
var devfileKUnstructured unstructured.Unstructured
devfileKUnstructured, err = libdevfile.GetK8sComponentAsUnstructured(a.Devfile, devfileK.Name, a.Context, devfilefs.DefaultFs{})
var devfileKUnstructuredList []unstructured.Unstructured
devfileKUnstructuredList, err = libdevfile.GetK8sComponentAsUnstructuredList(a.Devfile, devfileK.Name, a.Context, devfilefs.DefaultFs{})
if err != nil {
return nil, nil, fmt.Errorf("unable to read the resource: %w", err)
}
devfileK8sResourcesUnstructured = append(devfileK8sResourcesUnstructured, devfileKUnstructured)
devfileK8sResourcesUnstructured = append(devfileK8sResourcesUnstructured, devfileKUnstructuredList...)
}

isSBOSupported, err := a.kubeClient.IsServiceBindingSupported()
Expand Down
2 changes: 1 addition & 1 deletion pkg/kclient/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ type ClientInterface interface {
UpdateService(svc corev1.Service) (*corev1.Service, error)
ListServices(selector string) ([]corev1.Service, error)
DeleteService(serviceName string) error
GetOneService(componentName, appName string) (*corev1.Service, error)
GetOneService(componentName, appName string, isPartOfComponent bool) (*corev1.Service, error)
GetOneServiceFromSelector(selector string) (*corev1.Service, error)

// user.go
Expand Down
12 changes: 6 additions & 6 deletions pkg/kclient/mock_Client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/kclient/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ func (c *Client) DeleteService(serviceName string) error {

// GetOneService retrieves the service with the given component and app name
// An error is thrown when exactly one service is not found for the selector.
func (c *Client) GetOneService(componentName, appName string) (*corev1.Service, error) {
selector := odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode, false)
func (c *Client) GetOneService(componentName, appName string, isPartOfComponent bool) (*corev1.Service, error) {
selector := odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode, isPartOfComponent)
return c.GetOneServiceFromSelector(selector)
}

Expand Down
54 changes: 41 additions & 13 deletions pkg/libdevfile/component_kubernetes_utils.go
Original file line number Diff line number Diff line change
@@ -1,29 +1,57 @@
package libdevfile

import (
"bytes"
"io"

"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/pkg/devfile/parser"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
"github.com/ghodss/yaml"
yaml3 "gopkg.in/yaml.v3"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// GetK8sComponentAsUnstructured parses the Inlined/URI K8s of the devfile K8s component
func GetK8sComponentAsUnstructured(devfileObj parser.DevfileObj, componentName string,
context string, fs devfilefs.Filesystem) (unstructured.Unstructured, error) {
// GetK8sComponentAsUnstructuredList parses the Inlined/URI K8s of the Devfile K8s component and returns a list of unstructured.Unstructured objects;
// List is returned here because it is possible to define multiple K8s resources against a single Devfile K8s component
func GetK8sComponentAsUnstructuredList(devfileObj parser.DevfileObj, componentName string,
context string, fs devfilefs.Filesystem) ([]unstructured.Unstructured, error) {

strCRD, err := GetK8sManifestWithVariablesSubstituted(devfileObj, componentName, context, fs)
strCRD, err := GetK8sManifestsWithVariablesSubstituted(devfileObj, componentName, context, fs)
if err != nil {
return unstructured.Unstructured{}, err
return nil, err
}

// convert the YAML definition into map[string]interface{} since it's needed to create dynamic resource
u := unstructured.Unstructured{}
if err = yaml.Unmarshal([]byte(strCRD), &u.Object); err != nil {
return unstructured.Unstructured{}, err
var uList []unstructured.Unstructured
// Use the decoder to correctly read file with multiple manifests
decoder := yaml3.NewDecoder(bytes.NewBufferString(strCRD))
for {
var decodeU unstructured.Unstructured
if err = decoder.Decode(&decodeU.Object); err != nil {
if err == io.EOF {
break
}
return nil, err
}

// Marshal the object's data so that it can be unmarshalled again into unstructured.Unstructured object
// We do this again because yaml3 "gopkg.in/yaml.v3" pkg is unable to properly unmarshal the data into an unstructured object
rawData, err := yaml3.Marshal(decodeU.Object)
if err != nil {
return nil, err
}

// Use "github.com/ghodss/yaml" pkg to correctly unmarshal the data into an unstructured object;
var u unstructured.Unstructured
if err = yaml.Unmarshal(rawData, &u.Object); err != nil {
return nil, err
}

uList = append(uList, u)

}
return u, nil
return uList, nil
}

// ListKubernetesComponents lists all the kubernetes components from the devfile
Expand All @@ -34,14 +62,14 @@ func ListKubernetesComponents(devfileObj parser.DevfileObj, path string) (list [
if err != nil {
return
}
var u unstructured.Unstructured
var u []unstructured.Unstructured
for _, kComponent := range components {
if kComponent.Kubernetes != nil {
u, err = GetK8sComponentAsUnstructured(devfileObj, kComponent.Name, path, devfilefs.DefaultFs{})
u, err = GetK8sComponentAsUnstructuredList(devfileObj, kComponent.Name, path, devfilefs.DefaultFs{})
if err != nil {
return
}
list = append(list, u)
list = append(list, u...)
}
}
return
Expand Down
6 changes: 3 additions & 3 deletions pkg/libdevfile/libdevfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func IsDebugPort(name string) bool {

// GetContainerComponentsForCommand returns the list of container components that would get used if the specified command runs.
func GetContainerComponentsForCommand(devfileObj parser.DevfileObj, cmd v1alpha2.Command) ([]string, error) {
//No error if cmd is empty
// No error if cmd is empty
if reflect.DeepEqual(cmd, v1alpha2.Command{}) {
return nil, nil
}
Expand Down Expand Up @@ -416,12 +416,12 @@ func GetContainerComponentsForCommand(devfileObj parser.DevfileObj, cmd v1alpha2
}
}

// GetK8sManifestWithVariablesSubstituted returns the full content of either a Kubernetes or an Openshift
// GetK8sManifestsWithVariablesSubstituted returns the full content of either a Kubernetes or an Openshift
// Devfile component, either Inlined or referenced via a URI.
// No matter how the component is defined, it returns the content with all variables substituted
// using the global variables map defined in `devfileObj`.
// An error is returned if the content references an invalid variable key not defined in the Devfile object.
func GetK8sManifestWithVariablesSubstituted(devfileObj parser.DevfileObj, devfileCmpName string,
func GetK8sManifestsWithVariablesSubstituted(devfileObj parser.DevfileObj, devfileCmpName string,
context string, fs devfilefs.Filesystem) (string, error) {

components, err := devfileObj.Data.GetComponents(common.DevfileOptions{FilterByName: devfileCmpName})
Expand Down
10 changes: 5 additions & 5 deletions pkg/libdevfile/libdevfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ func TestGetDebugEndpointsForComponent(t *testing.T) {
}
}

func TestGetK8sManifestWithVariablesSubstituted(t *testing.T) {
func TestGetK8sManifestsWithVariablesSubstituted(t *testing.T) {
fakeFs := devfileFileSystem.NewFakeFs()
cmpName := "my-cmp-1"
for _, tt := range []struct {
Expand Down Expand Up @@ -1258,14 +1258,14 @@ func TestGetK8sManifestWithVariablesSubstituted(t *testing.T) {
return
}

got, err := GetK8sManifestWithVariablesSubstituted(tt.devfileObjFunc(), cmpName, "", fakeFs)
got, err := GetK8sManifestsWithVariablesSubstituted(tt.devfileObjFunc(), cmpName, "", fakeFs)
if (err != nil) != tt.wantErr {
t.Errorf("GetK8sManifestWithVariablesSubstituted() error = %v, wantErr %v",
t.Errorf("GetK8sManifestsWithVariablesSubstituted() error = %v, wantErr %v",
err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("GetK8sManifestWithVariablesSubstituted() got = %v, want %v",
t.Errorf("GetK8sManifestsWithVariablesSubstituted() got = %v, want %v",
got, tt.want)
}
})
Expand Down Expand Up @@ -1623,7 +1623,7 @@ func TestValidateAndGetPushCommands(t *testing.T) {
buildCommand: emptyString,
runCommand: "customcommand",
execCommands: execCommands,
//only the specified run command is returned, because the build command is not marked as default
// only the specified run command is returned, because the build command is not marked as default
numberOfCommands: 2,
wantErr: false,
},
Expand Down
Loading