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

Filter unknown custom resources in Delete- and ToggleTask #1552

Merged
merged 4 commits into from
Jun 9, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 27 additions & 33 deletions pkg/engine/renderer/enhancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
// and annotations
// it also takes care of setting an owner of all the resources to the provided object
type Enhancer interface {
Apply(templates map[string]string, metadata Metadata) ([]runtime.Object, error)
Apply(objs []runtime.Object, metadata Metadata) ([]runtime.Object, error)
}

// DefaultEnhancer is implementation of Enhancer that applies the defined conventions by directly editing runtime.Objects (Unstructured).
Expand All @@ -31,47 +31,41 @@ type DefaultEnhancer struct {

// Apply accepts templates to be rendered in kubernetes and enhances them with our own KUDO conventions
// These include the way we name our objects and what labels we apply to them
func (de *DefaultEnhancer) Apply(templates map[string]string, metadata Metadata) (objsToAdd []runtime.Object, err error) {
unstructuredObjs := make([]*unstructured.Unstructured, 0, len(templates))
func (de *DefaultEnhancer) Apply(sourceObjs []runtime.Object, metadata Metadata) ([]runtime.Object, error) {
unstructuredObjs := make([]*unstructured.Unstructured, 0, len(sourceObjs))

for name, v := range templates {
parsed, err := YamlToObject(v)
for _, obj := range sourceObjs {
unstructMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
if err != nil {
return nil, fmt.Errorf("%wparsing YAML from %s: %v", engine.ErrFatalExecution, name, err)
return nil, fmt.Errorf("%wconverting to unstructured failed: %v", engine.ErrFatalExecution, err)
}
for _, obj := range parsed {
unstructMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
if err != nil {
return nil, fmt.Errorf("%wconverting to unstructured failed: %v", engine.ErrFatalExecution, err)
}

objUnstructured := &unstructured.Unstructured{Object: unstructMap}
objUnstructured := &unstructured.Unstructured{Object: unstructMap}

if err = addLabels(objUnstructured, metadata); err != nil {
return nil, fmt.Errorf("%wadding labels on parsed object: %v", engine.ErrFatalExecution, err)
}
if err = addAnnotations(objUnstructured, metadata); err != nil {
return nil, fmt.Errorf("%wadding annotations on parsed object %s: %v", engine.ErrFatalExecution, obj.GetObjectKind(), err)
}
if err = addLabels(objUnstructured, metadata); err != nil {
return nil, fmt.Errorf("%wadding labels on parsed object: %v", engine.ErrFatalExecution, err)
}
if err = addAnnotations(objUnstructured, metadata); err != nil {
return nil, fmt.Errorf("%wadding annotations on parsed object %s: %v", engine.ErrFatalExecution, obj.GetObjectKind(), err)
}

isNamespaced, err := resource.IsNamespacedObject(obj, de.Discovery)
if err != nil {
return nil, fmt.Errorf("failed to determine if object %s is namespaced: %v", obj.GetObjectKind(), err)
}
isNamespaced, err := resource.IsNamespacedObject(obj, de.Discovery)
if err != nil {
return nil, fmt.Errorf("failed to determine if object %s is namespaced: %v", obj.GetObjectKind(), err)
}

// Note: Cross-namespace owner references are disallowed by design. This means:
// 1) Namespace-scoped dependents can only specify owners in the same namespace, and owners that are cluster-scoped.
// 2) Cluster-scoped dependents can only specify cluster-scoped owners, but not namespace-scoped owners.
// More: https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/
if isNamespaced {
objUnstructured.SetNamespace(metadata.InstanceNamespace)
if err := controllerutil.SetControllerReference(metadata.ResourcesOwner, objUnstructured, de.Scheme); err != nil {
return nil, fmt.Errorf("%wsetting controller reference on parsed object %s: %v", engine.ErrFatalExecution, obj.GetObjectKind(), err)
}
// Note: Cross-namespace owner references are disallowed by design. This means:
// 1) Namespace-scoped dependents can only specify owners in the same namespace, and owners that are cluster-scoped.
// 2) Cluster-scoped dependents can only specify cluster-scoped owners, but not namespace-scoped owners.
// More: https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/
if isNamespaced {
objUnstructured.SetNamespace(metadata.InstanceNamespace)
if err := controllerutil.SetControllerReference(metadata.ResourcesOwner, objUnstructured, de.Scheme); err != nil {
return nil, fmt.Errorf("%wsetting controller reference on parsed object %s: %v", engine.ErrFatalExecution, obj.GetObjectKind(), err)
}

unstructuredObjs = append(unstructuredObjs, objUnstructured)
}

unstructuredObjs = append(unstructuredObjs, objUnstructured)
}

if err := de.addDependenciesHashes(unstructuredObjs); err != nil {
Expand Down
42 changes: 12 additions & 30 deletions pkg/engine/renderer/enhancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/kubectl/pkg/scheme"
clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/yaml"

"github.com/kudobuilder/kuttl/pkg/test/utils"

Expand All @@ -27,8 +26,8 @@ import (

func TestEnhancerApply_embeddedMetadataStatefulSet(t *testing.T) {

tpls := map[string]string{
"deployment": resourceAsString(statefulSet("sfs1", "default")),
tpls := []runtime.Object{
statefulSet("sfs1", "default"),
}

meta := metadata()
Expand Down Expand Up @@ -72,8 +71,8 @@ func TestEnhancerApply_embeddedMetadataStatefulSet(t *testing.T) {

func TestEnhancerApply_embeddedMetadataCronjob(t *testing.T) {

tpls := map[string]string{
"cron": resourceAsString(cronjob("cronjob", "default")),
tpls := []runtime.Object{
cronjob("cronjob", "default"),
}

meta := metadata()
Expand Down Expand Up @@ -113,9 +112,9 @@ func TestEnhancerApply_embeddedMetadataCronjob(t *testing.T) {

func TestEnhancerApply_noAdditionalMetadata(t *testing.T) {

tpls := map[string]string{
"pod": resourceAsString(pod("pod", "default")),
"crd": resourceAsString(unstructuredCrd("crd", "default")),
tpls := []runtime.Object{
pod("pod", "default"),
unstructuredCrd("crd", "default"),
}

meta := metadata()
Expand Down Expand Up @@ -157,9 +156,7 @@ func TestEnhancerApply_noAdditionalMetadata(t *testing.T) {
func TestEnhancerApply_dependencyHash_noDependencies(t *testing.T) {
ss := statefulSet("statefulset", "default")

tpls := map[string]string{
"statefulset": resourceAsString(ss),
}
tpls := []runtime.Object{ss}

meta := metadata()
meta.PlanUID = uuid.NewUUID()
Expand Down Expand Up @@ -202,9 +199,7 @@ func TestEnhancerApply_dependencyHash_unavailableResource(t *testing.T) {
},
})

tpls := map[string]string{
"statefulset": resourceAsString(ss),
}
tpls := []runtime.Object{ss}

meta := metadata()
meta.PlanUID = uuid.NewUUID()
Expand Down Expand Up @@ -250,9 +245,7 @@ func TestEnhancerApply_dependencyHash_calculatedOnResourceWithoutLastAppliedConf
},
})

tpls := map[string]string{
"statefulset": resourceAsString(ss),
}
tpls := []runtime.Object{ss}

meta := metadata()
meta.PlanUID = uuid.NewUUID()
Expand Down Expand Up @@ -295,10 +288,7 @@ func TestEnhancerApply_dependencyHash_changes(t *testing.T) {
},
})

tpls := map[string]string{
"statefulset": resourceAsString(ss),
"configmap": resourceAsString(cm),
}
tpls := []runtime.Object{ss, cm}

meta := metadata()
meta.PlanUID = uuid.NewUUID()
Expand Down Expand Up @@ -327,10 +317,7 @@ func TestEnhancerApply_dependencyHash_changes(t *testing.T) {
assert.NotNil(t, hash, "Pod template spec annotations contains no dependency hash field")

cm.Data["newkey"] = "newvalue"
tpls = map[string]string{
"statefulset": resourceAsString(ss),
"configmap": resourceAsString(cm),
}
tpls = []runtime.Object{ss, cm}

objs, err = e.Apply(tpls, meta)
assert.Nil(t, err)
Expand Down Expand Up @@ -380,11 +367,6 @@ func owner() *corev1.Pod {
}
}

func resourceAsString(resource runtime.Object) string {
bytes, _ := yaml.Marshal(resource)
return string(bytes)
}

func configMap(name string, namespace string) *corev1.ConfigMap {
configMap := &corev1.ConfigMap{
TypeMeta: metav1.TypeMeta{
Expand Down
38 changes: 32 additions & 6 deletions pkg/engine/resource/object_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,33 +38,59 @@ func IsNamespacedObject(r runtime.Object, di discovery.CachedDiscoveryInterface)
return isNamespaced(gvk, di)
}

func IsKnownObjectType(r runtime.Object, di discovery.CachedDiscoveryInterface) (bool, error) {
gvk := r.GetObjectKind().GroupVersionKind()
return isKnownType(gvk, di)
}

// isNamespaced method return true if given runtime.Object is a namespaced (not cluster-scoped) resource. It uses the
// discovery client to fetch all API resources (with Groups and Versions), searches for a resource with the passed GVK
// and returns true if it's namespaced. Method returns an error if passed GVK wasn't found in the discovered resource list.
func isNamespaced(gvk schema.GroupVersionKind, di discovery.CachedDiscoveryInterface) (bool, error) {
// Fetch namespaced API resources
apiResource, err := getUncachedAPIResource(gvk, di)
if err != nil {
return false, err
}
if apiResource != nil {
return apiResource.Namespaced, nil
}
return false, fmt.Errorf("a resource with GVK %v seems to be missing in API resource list", gvk)
}

func isKnownType(gvk schema.GroupVersionKind, di discovery.CachedDiscoveryInterface) (bool, error) {
apiResource, err := getUncachedAPIResource(gvk, di)
if err != nil {
return false, err
}
if apiResource != nil {
return true, nil
}
return false, nil
}

// getUncachedAPIResource tries to invalidate the cache and requery the discovery interface to make sure no stale data is returned
func getUncachedAPIResource(gvk schema.GroupVersionKind, di discovery.CachedDiscoveryInterface) (*metav1.APIResource, error) {
// First try, this may return nil because of the cache
apiResource, err := getAPIResource(gvk, di)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess, it's a good thing that the discovery client is cached 😉

if err != nil {
return false, err
return nil, err
}
if apiResource != nil {
return apiResource.Namespaced, nil
return apiResource, nil
}

// Second try, now with invalidated cache. If we still get nil, we know it's not there.
log.Printf("Failed to get APIResource for %v, retry with invalidated cache.", gvk)
di.Invalidate()
apiResource, err = getAPIResource(gvk, di)
Copy link
Contributor

@zen-dog zen-dog Jun 8, 2020

Choose a reason for hiding this comment

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

Wdyt about making these next few lines a nested known := func(..) method? The same goes for the above isNamespaced/namespaced method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about the inline func, I tried it and it didn't really help much. But you're right that there's a bit of duplication, so I extracted a getUncachedAPIResource

if err != nil {
return false, err
return nil, err
}
if apiResource != nil {
return apiResource.Namespaced, nil
return apiResource, nil
}

return false, fmt.Errorf("a resource with GVK %v seems to be missing in API resource list", gvk)
return nil, nil
}

// getAPIResource returns a specific APIResource from the DiscoveryInterface or nil if no resource was found.
Expand Down
23 changes: 19 additions & 4 deletions pkg/engine/task/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,25 @@ func render(resourceNames []string, ctx Context) (map[string]string, error) {
return resources, nil
}

// enhance method takes a slice of rendered templates, applies conventions using Enhancer and
// returns a slice of k8s objects.
func enhance(rendered map[string]string, meta renderer.Metadata, enhancer renderer.Enhancer) ([]runtime.Object, error) {
enhanced, err := enhancer.Apply(rendered, meta)
// convert takes a map of rendered yaml templates and converts them to k8s objects
func convert(rendered map[string]string) ([]runtime.Object, error) {
objs := make([]runtime.Object, 0, len(rendered))

for name, v := range rendered {
parsed, err := renderer.YamlToObject(v)
if err != nil {
return nil, fmt.Errorf("%wparsing YAML from %s: %v", engine.ErrFatalExecution, name, err)
}
objs = append(objs, parsed...)
}

return objs, nil
}

// enhance method takes a slice of rendered k8s objects, applies conventions using Enhancer and
// returns a slice of enhanced k8s objects.
func enhance(objs []runtime.Object, meta renderer.Metadata, enhancer renderer.Enhancer) ([]runtime.Object, error) {
enhanced, err := enhancer.Apply(objs, meta)

switch {
case errors.Is(err, engine.ErrFatalExecution):
Expand Down
8 changes: 7 additions & 1 deletion pkg/engine/task/task_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ func (at ApplyTask) Run(ctx Context) (bool, error) {
return false, fatalExecutionError(err, taskRenderingError, ctx.Meta)
}

// 2. - Convert to objects
objs, err := convert(rendered)
if err != nil {
return false, fatalExecutionError(err, taskRenderingError, ctx.Meta)
}

// 2. - Enhance them with metadata -
enhanced, err := enhance(rendered, ctx.Meta, ctx.Enhancer)
enhanced, err := enhance(objs, ctx.Meta, ctx.Enhancer)
if err != nil {
return false, err
}
Expand Down
16 changes: 4 additions & 12 deletions pkg/engine/task/task_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,26 +198,18 @@ func resourceAsString(resource metav1.Object) string {

type testEnhancer struct{}

func (k *testEnhancer) Apply(templates map[string]string, metadata renderer.Metadata) ([]runtime.Object, error) {
result := make([]runtime.Object, 0)
for _, t := range templates {
objsToAdd, err := renderer.YamlToObject(t)
if err != nil {
return nil, fmt.Errorf("error parsing kubernetes objects after applying enhance: %w", err)
}
result = append(result, objsToAdd[0])
}
return result, nil
func (k *testEnhancer) Apply(objs []runtime.Object, metadata renderer.Metadata) ([]runtime.Object, error) {
return objs, nil
}

type fatalErrorEnhancer struct{}

func (k *fatalErrorEnhancer) Apply(templates map[string]string, metadata renderer.Metadata) ([]runtime.Object, error) {
func (k *fatalErrorEnhancer) Apply(objs []runtime.Object, metadata renderer.Metadata) ([]runtime.Object, error) {
return nil, fmt.Errorf("%wsomething fatally bad happens every time", engine.ErrFatalExecution)
}

type transientErrorEnhancer struct{}

func (k *transientErrorEnhancer) Apply(templates map[string]string, metadata renderer.Metadata) ([]runtime.Object, error) {
func (k *transientErrorEnhancer) Apply(objs []runtime.Object, metadata renderer.Metadata) ([]runtime.Object, error) {
return nil, fmt.Errorf("something transiently bad happens every time")
}