Test-infra: Prow: lint shell file mode

[BenTheElder]

Shell scripts should be marked executable... 🤦‍♂️

[stevekuznetsov]

/area prow
/kind feature

Are you suggesting a plugin? Or a presubmit job?

[BenTheElder]

I'm not sure, I'd have to check how cheap it is to do via the API. A presubmit might be a little heavyweight for just "if a *.sh is created make sure the file-mode is at least exec for the user..." and currently at least plugins are easier to add to many repos.

If not, probably just another verify script that we copy all over /shrug

[BenTheElder]

/priority backlog
/help

[anwaradil]

Is help still needed with this issue?

[BenTheElder]

Yes, nobody has taken this on yet.

We have a couple options here, but basically we should come up with something cheap that lets us verify that shell files are actually exec.

It's not super important, but it might help catch some bugs.

[anwaradil]

I can work on it. What files should I look at in the repo? if you can give me some more information that would be great :)

[BenTheElder]

so either we should:

• create a new re-usable prowjob (https://github.com/kubernetes/test-infra/blob/master/prow/pod-utilities.md#writing-a-prowjob-that-uses-pod-utilities) that does this
• or create a plugin for prow/hook (https://github.com/kubernetes/test-infra/tree/master/prow/plugins#plugins) that can be enabled to do this, the golint plugin might be a starting point for that route

[matthyx]

@anwaradil have you already worked on that issue? do you need help? should I do it?

[anwaradil]

@matthyx I have not started working on this. Please feel free to grab it. I appreciate your help.

[matthyx]

/assign
/remove-help

[matthyx]

@BenTheElder how about using the trees API?
example: https://api.github.com/repos/kubernetes/test-infra/git/trees/5c3f6549f19f53fcf7fd489e1ef2b60baef38a12

I can find the top dir sha with refs, and then create a prow job that loads all files recursively and verify that all that are named like *.sh and *.py have a mode of 100755?

[BenTheElder]

I can find the top dir sha with refs, and then create a prow job that loads all files recursively and verify that all that are named like *.sh and *.py have a mode of 100755?

Probably we could just skip to always creating a prow presubmit then? Not sure what the trees API is solving? 🤔

[matthyx]

Not sure what the trees API is solving?

It allows to see all files from the repo, along with their mode:

    {
      "path": ".bazelrc",
      "mode": "100644",
      "type": "blob",
      "sha": "9ecf28fe4d8d3ac5da8104d79b787c09ad4f0fb9",
      "size": 511,
      "url": "https://api.github.com/repos/kubernetes/test-infra/git/blobs/9ecf28fe4d8d3ac5da8104d79b787c09ad4f0fb9"
    },

[BenTheElder]

Ah I see.
A ProwJob can also just checkout the repo and list the files though? A re-usable prowjob config might be simpler and avoid extra API token usage.

With the podutils the config for this could probably be just a few lines if we published the linter in an image.

[matthyx]

Alright, lemme investigate your page (https://github.com/kubernetes/test-infra/blob/master/prow/pod-utilities.md#writing-a-prowjob-that-uses-pod-utilities)

[matthyx]

I can use the alpine image and simply find those files with a find shell-fu like that:

~/workspace/go/src/k8s.io/test-infra$ find . -name "*.sh" \! -perm /a+x -ls
  3015645      4 -rw-rw-r--   1 mbertsch mbertsch     1298 Jan  9 10:05 ./planter/install-bazel.sh
  3015081      4 -rw-rw-r--   1 mbertsch mbertsch     1858 Jan 28 08:15 ./experiment/kind-coverage-dump.sh
  3015440      4 -rw-rw-r--   1 mbertsch mbertsch     1312 Jan  9 10:05 ./images/kubekins-e2e/install-bazel.sh

[matthyx]

How do you report errors with prowjobs? Is it just a matter of returning a non-0 code?

[BenTheElder]

Non-zero return code and ideally some output in the log identifying what is
wrong.

Depending on the complexity it might be worth packaging a small program
into a tiny image so the prowjob can just be setting the image and command.

On Mon, Feb 18, 2019, 23:23 Matthias Bertschy <[email protected]
wrote:

How do you report errors with prowjobs? Is it just a matter of returning a
non-0 code?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/issues/9417#issuecomment-465016025,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4BqyU0v1Q7jHIeOUcaL0tHejFI9aw-ks5vO6aIgaJpZM4WqLdz
.

[matthyx]

Ok, ideally it would reside in config/jobs/kubernetes/test-infra/test-infra-presubmits.yaml ?

[BenTheElder]

yes :+1:, thanks!