earthobservations / wetterdienst

Open weather data for humans.
https://wetterdienst.readthedocs.io/
MIT License
347 stars 54 forks source link

Handle external pull requests wrt. OCI image build + push well #947

Closed amotl closed 1 year ago

amotl commented 1 year ago

Hi there,

at https://github.com/earthobservations/wetterdienst/pull/945#issuecomment-1567539560, @neumann-nico is reporting upon a minor but not unimportant quirk with the CI setup.

The problem is that external contributors, including Dependabot, does not have the permissions to push to GHCR, the OCI image registry of GitHub.

@gutzbenj @amotl Dependabot has not the rights to run all CI tests.

Release Docker Full/Standard are failing with: Error: buildx failed with: ERROR: failed to solve: failed to push ghcr.io/earthobservations/wetterdienst-full:pr-945: unexpected status: 403 Forbidden

We could either

  1. Give him the rights
  2. Skip the docker pushing for dependabot
  3. Someone with the rights needs tor run the tests

Instead of discussing on a pull request, let's discuss here.

With kind regards, Andreas.

amotl commented 1 year ago

Hi Nico,

Skip the "push" step for Dependabot.

I've tried it on two other spots already, but haven't been successful so far.

With kind regards, Andreas.

amotl commented 1 year ago

@gutzbenj said at https://github.com/earthobservations/wetterdienst/pull/945#issuecomment-1567963571:

This is also effecting anyone who's not part of our organization and would open a PR from their fork e.g. max-mustermann/pr-to-wetterdienst. Ignoring would be fine but will have to figure the correct filter for PR branches.

amotl commented 1 year ago

It does not happen so often, but it's true. I wonder how others are handling this.

One idea: Build up an allow-list of contributors who have the corresponding permissions, and add a condition to the build+push workflow step to run only if the contribution is coming from someone who is member of this allow-list, and skip it for all others?

The good thing with our OCI building workflow recipes is that the build_and_test job will already cover the building of the image thoroughly, otherwise this operation would be complete nonsense at all.

gutzbenj commented 1 year ago

I think the allow list wouldn't work because there's no way to leak our GCR secret to someone. This is simply not the idea organization secrets.

amotl commented 1 year ago

I think the allow list wouldn't work because there's no way to leak our GCR secret to someone. This is simply not the idea organization secrets.

I don't understand this comment about the GCR secret and the organization secrets. Can you clarify?

My proposal was to only run the build&upload job based on an allow-list, so that it would only run when the patch originates from some of us who have the permissions, and skip it for all others.

Do you have a different solution in mind? We can also ask GitHub support for help, if we can't discover how others are doing it.

gutzbenj commented 1 year ago

Sorry for the late response: This was fixed with https://github.com/earthobservations/wetterdienst/blob/de0b1e546e18c5085b41ed3806d6f937de3bf623/.github/workflows/docker-publish-standard.yml#L31

Same for the full version