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

Feature: Use YAML instead of Kubernetes as validation source #65

Closed
estahn opened this issue Apr 30, 2020 · 22 comments
Closed

Feature: Use YAML instead of Kubernetes as validation source #65

estahn opened this issue Apr 30, 2020 · 22 comments
Assignees
Labels
enhancement New feature or request Priority/Medium
Milestone

Comments

@estahn
Copy link

estahn commented Apr 30, 2020

The tool currently checks objects in Kubernetes, which is great. I would also like to add it to our CI/CD to check manifests before they make it to Kubernetes. In that regards, I'd like to either pipe YAML/JSON into the tool and it provides the same output including error codes for the pipeline to fail.

Let me know your thoughts.

@rikatz rikatz self-assigned this Apr 30, 2020
@rikatz rikatz added enhancement New feature or request Priority/Medium labels Apr 30, 2020
@rikatz rikatz added this to the v1.0.0 milestone Apr 30, 2020
@rikatz
Copy link
Collaborator

rikatz commented Apr 30, 2020

@estahn thanks for raising this feature request :D

I'm putting this into the v1.0.0 release, which will bring also json/yaml output (tks to @chentex ) and will work into this feature during the weekend.

@rikatz
Copy link
Collaborator

rikatz commented May 4, 2020

Hi @estahn

Didn't got enough time this weekend to finish this, I'm going to cut a release here with the output formatter, and this item is the next in my backlog :)

@estahn
Copy link
Author

estahn commented May 4, 2020

Sounds good, thanks for the update :)

@rikatz rikatz modified the milestones: v1.0.1, v1.0.2 May 5, 2020
@rikatz
Copy link
Collaborator

rikatz commented May 5, 2020

@estahn I'm moving this to the next milestone as this might be more tricky than I figured out :) But no worries, soon or later (I hope sooner than later) this will get it into kubepug.

@rikatz
Copy link
Collaborator

rikatz commented May 10, 2020

Writing some use cases here so I won't forget:

  • User supplies a single file, with a single object:
version: apps/v1
kind: Deployment
[....]
  • User supplies a single file with multiple objects:
version: apps/v1
kind: Deployment
[...]
---
version: v1
kind: Pod
[...]
  • User supplies a dir with multiple yaml files, with both cases as the above. Question here: should the program support and try to unmarshall all files in a directory, skipping files that it cannot understand or should it look into specific extension, as object.yaml.

Edit: Question: Do we wan't to deal also with subdirs inside a dir? Should this use a "--recursive" flag?

  • The same case of all above, but with json. In this case, it would be easier to supply a --input-format other than trying to figure out what's the input format, but need to evolve this.

@estahn Do you remember another scenario?

@estahn
Copy link
Author

estahn commented May 12, 2020

@rikatz I think the most likely is a single file with multiple objects, e.g. kustomize or helm template.

@rikatz
Copy link
Collaborator

rikatz commented May 17, 2020

Started this in branch https://github.com/rikatz/kubepug/tree/input-files

@estahn
Copy link
Author

estahn commented May 26, 2020

@rikatz How is it going with the input files? Do you need some help?

@rikatz
Copy link
Collaborator

rikatz commented May 26, 2020

Hi @estahn :)

I’m pretty crowded with work here, that’s why I’m not taking care of this issue right now ;(

I was also thinking about using this to refactor some parts of the code but if you got some PR already in mind or wanna help with this one please feel free to open the PR :)

Thank you :)

@rikatz
Copy link
Collaborator

rikatz commented May 30, 2020

@estahn as the above issue, I've found some logics issues that will demand some program rewriting to allow other sorts of input. This can be followed in #77

As this is ready and I really hope this doesn't take so much time, it will be much easier to implement whatever input reader for Kubepug, being file the first but also, I don't know maybe reading from a GitRepo, etc, as this is going to became only a Interface and each object will have it's type of comparision.

@rikatz
Copy link
Collaborator

rikatz commented Jun 8, 2020

@estahn Hello 👋

I've finished the main refactoring and also put the file parser to work, it's in https://github.com/rikatz/kubepug/tree/refactor-1

Can you please compile this and make some tests? I still have to perform some cleanup in the code, and the flags might change.

I'll also write in a near future a form to detect if the input file is a YAML or a JSON, but right now it only supports YAML (you can pass with --input-file a specific file or a directory containing multiple files, it will skip invalid files (if a file contain multiple docs and one is invalid it will skip the whole file)

@estahn
Copy link
Author

estahn commented Jun 8, 2020

@rikatz Looks good, see script below to run it on the stable charts. Would be good to change exit code if failure are found so it can be used to fail CI.

#!/bin/bash

charts=$(curl -sSL https://api.github.com/repos/helm/charts/contents/stable  | jq -r '.[].name')

for chart in $charts; do
  echo "----------------- CHART $chart ---------------"
  helm template test stable/$chart > test.yaml
  go run cmd/kubepug.go --input-file test.yaml --k8s-version v1.16.10
  echo "EXIT CODE $?"
done

@rikatz
Copy link
Collaborator

rikatz commented Jun 8, 2020

Try calling with —error-on-deprecated —error-on-deleted and it should return an Exit Code 1

@estahn
Copy link
Author

estahn commented Jun 8, 2020

@rikatz I get the following for some of them:

time="2020-06-08T22:35:54+10:00" level=warning msg="YAML file does not contain apiVersion or Kind: /tmp/buildkite.yaml  Skipping to next"

@rikatz
Copy link
Collaborator

rikatz commented Jun 8, 2020

Can you please post the content of /tmp/buildkite.yaml?

As kubepug reads all of the files from the directory you're pointing into (it seems to be --input-file /tmp' it can parse buildkite.yaml as a YAML file but as it doesn't contains basic fields needed to check if the object is deprecated (apiVersion and Kind) it will issue a warning :)

@estahn
Copy link
Author

estahn commented Jun 8, 2020

WARNING: This chart is deprecated
---
# Source: buildkite/templates/service-account.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  name: test-buildkite
  labels:
    app: buildkite
    chart: buildkite-0.2.4
    release: test
    heritage: Helm
---
# Source: buildkite/templates/secret.yaml
apiVersion: v1
kind: Secret
metadata:
  name: test-buildkite
  labels:
    app: buildkite
    chart: buildkite-0.2.4
    release: test
    heritage: Helm
type: Opaque
data:
  agent-token:

I adjusted my script as follows:

#!/bin/bash

charts=$(curl -sSL https://api.github.com/repos/helm/charts/contents/stable  | jq -r '.[].name')

for chart in $charts; do
  echo "----------------- CHART $chart ---------------"
  helm template test stable/$chart > /tmp/$chart.yaml
  go run cmd/kubepug.go --error-on-deprecated --error-on-deleted --k8s-version v1.16.10 --input-file /tmp/$chart.yaml
  echo "EXIT CODE $?"
done

@estahn
Copy link
Author

estahn commented Jun 8, 2020

@rikatz Ah, I guess it's for the first block which is a WARNING.

@rikatz
Copy link
Collaborator

rikatz commented Jun 8, 2020

exactly :)

The thing is that I didn't realized yet how to skip only a block and not the whole YAML file, as the yaml parser deals with multi document in a different way. Will have to dig a little bit deeper, but right now if you have a YAML file with multiple documents and one not valid to kubernetes the whole file will be ignored :)

I'll probably make some further minor improvements this week and cut a beta release

@rikatz
Copy link
Collaborator

rikatz commented Jun 8, 2020

Well, got an idea of maybe convert the whole file to the struct and then deal with the invalid entries in the struct.

Will try that later

@rikatz
Copy link
Collaborator

rikatz commented Jun 14, 2020

Fixed in #78

I'm opening a new issue for now just to deal better with the YAML parser. I've taken a look into the parser from the kubectl program and the behavior is the same: if you have a file with multiple documents and one is invalid, it halts the processing of everything else so because of the yaml parser (the go library) it might be a bit complicated to deal with that. But I'll open that issue so I don't forget :)

@rikatz rikatz closed this as completed Jun 14, 2020
@rikatz rikatz mentioned this issue Jun 14, 2020
@estahn
Copy link
Author

estahn commented Jun 17, 2020

@rikatz Do you want to cut a new release so we can start using the yaml input?

@rikatz
Copy link
Collaborator

rikatz commented Jun 17, 2020

Yes, I'll do this today :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Priority/Medium
Projects
None yet
Development

No branches or pull requests

2 participants