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

feat(process): Unwrap List types #306

Merged
merged 2 commits into from
Jul 1, 2020
Merged

feat(process): Unwrap List types #306

merged 2 commits into from
Jul 1, 2020

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Jun 30, 2020

This PR adds native support to Tanka for unwrapping List objects, those
being k8s pseudo objects used to bundle multiple resources into one
object.

To detect whether something is a List or not, the existence of items
is used, the same criteria the offical k8s.io packages use:

// https://github.com/kubernetes/apimachinery/blob/61490fe38e784592212b24b9878306b09be45ab0/pkg/apis/meta/v1/unstructured/unstructured.go#L54-L61
func (obj *Unstructured) IsList() bool {
	field, ok := obj.Object["items"]
	if !ok {
		return false
	}
	_, ok = field.([]interface{})
	return ok
}

Also refactors SchemaError which is now used in Unwrap as well. The error now also displays a code snippet, so debugging becomes easier

Fixes #277

This PR adds native support to Tanka for unwrapping List objects, those
being k8s pseudo objects used to bundle multiple resources into one
object.

To detect whether something is a List or not, the existence of `items`
is used, the same criteria the offical `k8s.io` packages use:

https://github.com/kubernetes/apimachinery/blob/61490fe38e784592212b24b9878306b09be45ab0/pkg/apis/meta/v1/unstructured/unstructured.go#L54
@sh0rez sh0rez added kind/feature Something new should be added component/kubernetes Working with Kubernetes clusters labels Jun 30, 2020
@sh0rez sh0rez self-assigned this Jun 30, 2020
Now always gives name, reason and a code sample of the violating object
@sh0rez sh0rez marked this pull request as ready for review July 1, 2020 12:17
Copy link
Contributor

@malcolmholmes malcolmholmes left a comment

Choose a reason for hiding this comment

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

Tried this out.

Added this out first:

  list: {
    kind: 'List',
    apiVersion: 'v1',
    items: [{
      kind: 'Deployment',
      apiVersion: 'apps/v1',
    }],
  },

It complained:

.list.items[0] has missing or invalid fields: metadata, metadata.name:

  apiVersion: apps/v1
  kind: Deployment

Please check above object.

This complaint seems completely reasonable. I followed its instructions:

  list: {
    kind: 'List',
    apiVersion: 'v1',
    items: [{
      kind: 'Deployment',
      apiVersion: 'apps/v1',
      metadata: {
        name: 'foo',
      },
    }],
  },

This rendered as expected, the deployment was in the list of resources, and there was no list in the output.

Copy link
Contributor

@mplzik mplzik left a comment

Choose a reason for hiding this comment

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

LGTM, but note that this also does refactoring of SchemaError that is not mentioned in the description; might be worth adding it to the description.

@sh0rez sh0rez merged commit 255666d into master Jul 1, 2020
@sh0rez sh0rez deleted the unwrap branch July 1, 2020 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/kubernetes Working with Kubernetes clusters kind/feature Something new should be added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

injectLabels should not inject on RoleList and RoleBindingList objects
3 participants