Fix ci permissions

[colearendt]

What this PR does / why we need it:

The Apache Foundation has guidance that was missed on my first PR for GitHub Actions usage. This PR addresses that feedback.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

• fixes #62

Special notes for your reviewer:

A couple of questions:

• Regarding the contention for GitHub Actions usage - did you see any concerning contention yet? I'm wondering if the "install" action should be:

  • Combined with the lint job so that it will cut down on "redundant" setup (i.e. installing helm, etc.)
  • Split into a dedicated workflow and run it on demand (with a slash command or something like that)
  • I tend to lean towards the latter, if any modification is needed. If you can wait and see / make changes as needed, then that's probably preferable too. Just wanted to bring to your attention. (Not super costly at ~ 2 minutes)

• I used the persist-credentials: false convention recommended in those docs. Not sure if this will cause problems for the rebuild or release workflows (i.e. if they make implicit use of that token cache)

• I'm also not sure if the release script requires access to write packages. I don't think so, so I left this off. There does not seem to be a great way to test these scripts from a branch

• One more note for y'all's Apache docs. It is possible to set things within a repository to only trigger actions once approved by a maintainer. i.e. so changes to workflows files do not automatically get to run. This also cuts down on "duplicate" runs of workflows and could help alleviate any capacity issues that y'all have (if those are still a problem) at the cost of needing a maintainer to run the CI manually by approving

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.

• [ ] Chart Version bumped
• [ ] e2e tests pass
• [ ] Variables are documented in the README.md
• [ ] Chart tgz added to /docs and index updated

[colearendt]

A note about the way that CI is currently set up:

• on this PR, there are no chart changes
• as a result, the chart "CI" does not run ("no changes detected")

(so the "check" here is somewhat of a "false positive")

Would it be preferable to set it up so that if there are changes to the .github/workflows directory, the "all" lint/installs run? (Not sure how hard/tedious this is to detect off-hand...). An alternative way to test more thoroughly would be to create a dummy PR with a chart change (or add a commit that changes the chart and then remove it / revert it)

Alternatively, we could just have it run CI on the chart every time... or only trigger the CI if changes are in the chart directory / the .github/workflows or .github/actions folder (i.e. remove the "change detection" since you just have a single chart). This latter approach seems like a cleaner solution to me.

i.e. run "always" / remove conditional checks but only trigger the workflow, so it runs on every commit to main, but on PRs that change the chart or CI definitions :

on:
  push:
    branches:
      - main
  pull_request:
    paths:
      - '.github/workflows/chart-test.yaml'
      - '.github/actions/**'
      - 'couchdb/**'

EDIT: I talked myself into it and added to the PR 😅

[colearendt]

It looks like CI is happy... although I'll defer to you if you think there is anything we should improve about the logs here before we move on: https://github.com/apache/couchdb-helm/runs/6947632474?check_suite_focus=true

It seems like it succeeded... but I suspect there are some things that can be done long term to improve / ensure that we are picking up health of the service / etc. helm test is probably needed, as alluded to here. Maybe those are best as follow-ups?

[willholley]

or only trigger the CI if changes are in the chart directory / the .github/workflows or .github/actions folder (i.e. remove the "change detection" since you just have a single chart). This latter approach seems like a cleaner solution to me.

Agree this sounds cleaner. +1

[willholley]

Thanks for this follow up @colearendt. On helm test, is this not covered by the ct install --upgrade step?

[colearendt]

@willholley helm test is covered by the ct install --upgrade step. The trick is that the couchdb helm chart doesn't have any "test hooks" included in it. As a result, helm test currently does nothing - ct just checks that the install "worked". By writing test jobs / hooks, we can run couchdb specific checks (in Kubernetes jobs) to be sure that the service is up / happy / has a user provisioned, etc.