Skip to content

Commit

Permalink
Handle unknown types as unstructured (#954)
Browse files Browse the repository at this point in the history
**What this PR does / why we need it**:
When scheme did not contain that type, we previously failed. Since we should be able to handle all types that people might have installed in their cluster, we should fallback to parsing a type as unstructured when not available.

This PR has 3 parts:
- update to `ParseKubernetesObjects` to process unknown types as Unstructured
- use of dynamic restmapper in execution that ensures that we discover new types that were added during the lifetime of the controller
- integration test

NOTE: Right now we are using dynamic mapper from test harness. That is probably not as robust as the one introduced in the controller-runtime 0.3.0. I will port this code to the controller-runtime mapper when we bump the version of controller-runtime (that should happen prior to 0.8.0 anyway).

Fixes #913
  • Loading branch information
alenkacz committed Oct 18, 2019
1 parent a6e1755 commit 5459b7d
Show file tree
Hide file tree
Showing 14 changed files with 448 additions and 7 deletions.
5 changes: 4 additions & 1 deletion cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/kudobuilder/kudo/pkg/controller/instance"
"github.com/kudobuilder/kudo/pkg/controller/operator"
"github.com/kudobuilder/kudo/pkg/controller/operatorversion"
util "github.com/kudobuilder/kudo/pkg/test/utils"
"github.com/kudobuilder/kudo/pkg/version"
apiextenstionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"

Expand All @@ -40,7 +41,9 @@ func main() {

// create new controller-runtime manager
log.Info("setting up manager")
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{})
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
MapperProvider: util.NewDynamicRESTMapper,
})
if err != nil {
log.Error(err, "unable to start manager")
os.Exit(1)
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ require (
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3
golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a // indirect
golang.org/x/sys v0.0.0-20190911201528-7ad0cfa0b7b5 // indirect
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 // indirect
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
golang.org/x/tools v0.0.0-20190909030654-5b82db07426d
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7
google.golang.org/appengine v1.5.0 // indirect
google.golang.org/grpc v1.21.0 // indirect
gopkg.in/yaml.v2 v2.2.2
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ golang.org/x/tools v0.0.0-20190501045030-23463209683d/go.mod h1:RgjU9mgBXZiqYHBn
golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190909030654-5b82db07426d h1:PhtdWYteEBebOX7KXm4qkIAVSUTHQ883/2hRB92r9lk=
golang.org/x/tools v0.0.0-20190909030654-5b82db07426d/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gomodules.xyz/jsonpatch/v2 v2.0.1 h1:xyiBuvkD2g5n7cYzx6u2sxQvsAy4QJsZFCzGVdzOXZ0=
gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU=
Expand Down
3 changes: 2 additions & 1 deletion pkg/test/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ func (h *Harness) RunKUDO() error {
}

mgr, err := manager.New(config, manager.Options{
Scheme: testutils.Scheme(),
Scheme: testutils.Scheme(),
MapperProvider: testutils.NewDynamicRESTMapper,
})
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions pkg/test/utils/dynamicrestmapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ package utils
// This helps work around issues with the DiscoveryRESTMapper caching resources.

import (
"fmt"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime/schema"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -52,6 +54,7 @@ func (drm *DynamicRESTMapper) reloadOnError(err error) bool {
return false
}
err = drm.reload()
fmt.Println("reload")
if err != nil {
utilruntime.HandleError(err)
}
Expand Down
21 changes: 17 additions & 4 deletions pkg/util/template/template.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
package template

import (
"bytes"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"k8s.io/apimachinery/pkg/runtime"

yamlutil "k8s.io/apimachinery/pkg/util/yaml"
"k8s.io/client-go/kubernetes/scheme"
)

//ParseKubernetesObjects parses a list of runtime.Objects from the provided yaml
// ParseKubernetesObjects parses a list of runtime.Objects from the provided yaml
// If the type is not known in the scheme, it tries to parse it as Unstructured
// TODO(av) could we use something else than a global scheme here? Should we somehow inject it?
func ParseKubernetesObjects(yaml string) (objs []runtime.Object, err error) {
sepYamlfiles := strings.Split(yaml, "---")
for _, f := range sepYamlfiles {
Expand All @@ -21,10 +27,17 @@ func ParseKubernetesObjects(yaml string) (objs []runtime.Object, err error) {
obj, _, e := decode([]byte(f), nil, nil)

if e != nil {
err = e
return
// if parsing to scheme known types fails, just try to parse into unstructured
unstructuredObj := &unstructured.Unstructured{}
fileBytes := []byte(f)
decoder := yamlutil.NewYAMLOrJSONDecoder(bytes.NewBuffer(fileBytes), len(fileBytes))
if err = decoder.Decode(unstructuredObj); err != nil {
return nil, err
}
objs = append(objs, unstructuredObj)
} else {
objs = append(objs, obj)
}
objs = append(objs, obj)
}
return
}
24 changes: 24 additions & 0 deletions pkg/util/template/template_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package template

import "testing"

func TestParseKubernetesObjects_UnknownType(t *testing.T) {
_, err := ParseKubernetesObjects(`apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
labels:
app: prometheus-operator
release: prometheus-kubeaddons
name: spark-cluster-monitor
spec:
endpoints:
- interval: 5s
port: metrics
selector:
matchLabels:
spark/servicemonitor: true`)

if err != nil {
t.Errorf("Expecting no error but got %s", err)
}
}
4 changes: 4 additions & 0 deletions test/integration/operator-with-custom-crd/00-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: servicemonitors.monitoring.coreos.com
Loading

0 comments on commit 5459b7d

Please sign in to comment.