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

CI/CD friendliness #5

Closed
wind57 opened this issue Jul 3, 2021 · 6 comments
Closed

CI/CD friendliness #5

wind57 opened this issue Jul 3, 2021 · 6 comments

Comments

@wind57
Copy link

wind57 commented Jul 3, 2021

An interesting tool overall!

I do not see a way to use it in a CI/CD env, unless I am missing the obvious.

It would be great if it had something along the lines of --quiet and --error-count, so that I could parse the result of helm datree test ... and find our if I need to fail the build or not. Currently this is very cumbersome to do (I get the output and parse it, which to put it mildly, simply sucks).

Thank you for looking into this.

@wind57
Copy link
Author

wind57 commented Jul 3, 2021

oh! so I can pass -o json to helm datree... this simplifies things. Though the output is weird:

  "InvalidK8sFiles": [
    {
      "Path": "/tmp/manifest.yaml",
      "ValidationErrors": [
        {
          "ErrorMessage": "For field apiVersion: apiVersion must be one of the following: \"rbac.authorization.k8s.io/v1\""
        }
      ]
    }
  ]
  • The part that looks incorrect is : /tmp/manifest.yaml - that is not my path, neither my file. I guess you copy the manifests and then validate them and show that incorrect path.
  • And you also present this line : Error: plugin "datree" exited with error at the end; that is not part of the json itself.

This is what I currently ended up doing :

IFS=$'\n'
declare -a datree_result
datree_result=$(helm datree test src/main/helm -o json | jq -c 'paths | select(.[-1] == "ErrorMessage")')

if [[ "${datree_result[@]}" =~ "ErrorMessage" ]]
then
   echo "failure"
   exit 1
fi

@eyarz
Copy link
Member

eyarz commented Jul 20, 2021

@wind57 I forgot to put myself as a watcher to this repo, so I missed your issue - sorry about that 😞

I'm glad to see that you find a workaround, but I still want to answer your questions:

  • The way the plugin works is that it generates a Kubernetes manifest from your helm chart and running Datree on that auto-generated manifest. Because we need to generate a new file, we are giving it the name /tmp/manifest.yaml
  • TheError: plugin "datree" exited with error is an error code that is generated by helm (not by Datree), but we deployed a new version that is also handling it
  • We also support YAML output :)

I'm wondering why do parse Datree's output? If it fails, it should have exit code 1 anyway...

@dimabru
Copy link
Contributor

dimabru commented Jul 20, 2021

FYI, our latest update includes a small change to the way we generate these tmp files.
Since we added pipe support in datree cli, we can now forward helm template output directly to the datree cli. This results in a slightly different filename in tmp folder.
Should be something like /var/folders/4k/7h6k03k11rv8gy62py4fxqxh0000gm/T/datree_temp_130557047.yaml

@wind57
Copy link
Author

wind57 commented Jul 24, 2021

I'm wondering why do parse Datree's output? If it fails, it should have exit code 1 anyway...

Right. But what is the use of that to the people using this? I mean I would like to show a human readable error message of what went wrong and where. So I wanted to use -o json, so that I could parse it with jq and extract the error message(s).

It seems you validate errors vs "suggestions" differently, which makes my code really involved. Let me explain:

  • if I specify an invalid value as : apiVersion: rbac.authorization.k8s.io/v1123 in one of the files, and run: helm datree test src/main/helm -o json | jq, I would get a json that contains:
  "InvalidK8sFiles": [
    {
      "Path": "/tmp/manifest.yaml",
      "ValidationErrors": [
        {
          "ErrorMessage": "could not find schema for ClusterRole"
        }
      ]
    }
  ]

Good. I can check if the output has ErrorMessage and parse the output and get the proper error message and display that to the end user.

  • On the other hand, if I have valid manifests, but :
spec:
  type: NodePort
  ports:
  - port: 8080
    targetPort: 8080
    nodePort: 30008

I get a json that contains and run : helm datree test src/main/helm -o json | jq

"EvaluationResults": {
    "FileNameRuleMapper": {
      "/tmp/manifest.yaml": {
        "7": {
          "ID": 7,
          "Name": "Prevent Service from exposing node port",
          "FailSuggestion": "Incorrect value for key `type` - `NodePort` will open a port on all nodes where it can be reached by the network external to the cluster",
          "Count": 1
        }
      }
    }

Yeah, no ErrorMessage, so my code is now supposed to check in a various place for a different thing.

imho, a simpler type like:

"EvaluationResult" : {
     "Type" : "Suggestion/Error"
     "SuggestionIfPresent" : "....."
     ....
}

So my suggestion is a unified, single place, for errors. I hope this makes sense.

@dimabru as to your suggestion, you should really maintain a "mapping"... What I mean by that: if I have 20 manifest files in a single helm chart and I display the error being in /tmp/manifest.yaml, the end user looking at those errors is going to be in the blur. Where exactly is the problem?

Thank you.

@stale
Copy link

stale bot commented Aug 3, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@eyarz
Copy link
Member

eyarz commented Aug 4, 2021

The output is provided by Datree CLI and not the plugin so I open a new issue there.

@stale stale bot removed the stale issue label Aug 4, 2021
@eyarz eyarz closed this as completed Aug 4, 2021
paulbsac added a commit to paulbsac/helm-datree that referenced this issue Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants