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.

Sign up for GitHub

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

Jump to bottom

ci: build docker only when secrets are present #996

Merged
merged 1 commit into from
Jul 3, 2024
Merged

ci: build docker only when secrets are present #996

merged 1 commit into from
Jul 3, 2024

Conversation

iwpnd
Copy link
Member

@iwpnd iwpnd commented Jul 1, 2024

as discussed in #995 docker build should only run if secrets.DOCKERHUB_USERNAME and secrets.DOCKERHUB_TOKEN are present.

@iwpnd iwpnd requested review from gdey and ARolek as code owners July 1, 2024 19:08
@coveralls
Copy link

coveralls commented Jul 1, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 5117e2b44-PR-996

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build 86cf1919d-PR-992: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024

@iwpnd thanks for jumping on this! I think you updated the wrong action though. I believe you meant to update https://github.com/go-spatial/tegola/blob/master/.github/workflows/on_pr_push.yml but instead updated the release action.

@iwpnd
Copy link
Member Author

iwpnd commented Jul 1, 2024

The failing docker build is part of on_release. If you do not want to build at all in PRs we can also remove the

on:
  pull_request:

from the release workflow. In that case we can also skip the check on whether the secrets are defined, because it only runs after the PR is merged. @ARolek

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024

@iwpnd ah yes, you're right! I remember when this was added years ago. It had to do with errors popping up during release that didn't get caught during the PR. So let's leave the check. Your changes LGTM! 🙏

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024 •
edited
Loading

@iwpnd it appears we have a syntax error in the workflow file:

image

Edit: Workflow run: https://github.com/go-spatial/tegola/actions/runs/9751755854

@iwpnd
Copy link
Member Author

iwpnd commented Jul 1, 2024

@iwpnd it appears we have a syntax error in the workflow file:

image Edit: Workflow run: https://github.com/go-spatial/tegola/actions/runs/9751755854

I’ll take a look in the morning. Seems I don’t have access to secrets in the if statement after all. 🤔

sorry for the noise 🙏🏻

@ARolek
Copy link
Member

ARolek commented Jul 1, 2024

I’ll take a look in the morning. Seems I don’t have access to secrets in the if statement after all. 🤔
sorry for the noise 🙏🏻

No worries! I was hoping this worked as it was a clean solution.

@coveralls
Copy link

coveralls commented Jul 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 45fcda619-PR-996

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 2caf5013a-PR-996

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@iwpnd
Copy link
Member Author

iwpnd commented Jul 2, 2024

okay TIL i dont have access to secrets, but I can export secrets to the environment variables and use that instead. 😄

@ARolek now when we merge the secrets should be populated in those environment variables and the docker build should execute successfully and not be skipped.

@coveralls
Copy link

coveralls commented Jul 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 098825527-PR-996

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@ARolek
Copy link
Member

ARolek commented Jul 3, 2024

okay TIL i dont have access to secrets, but I can export secrets to the environment variables and use that instead.

Interesting! Still looks clean to me ;-)

ARolek
ARolek approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@ARolek ARolek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Can you please squash into a single commit?

@iwpnd iwpnd force-pushed the ci/docker-build-only-with-secrets branch from c3d2174 to aa8e390 Compare July 3, 2024 17:38
@coveralls
Copy link

coveralls commented Jul 3, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 6bb4ab97e-PR-996

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@iwpnd
ci: build docker only when secrets are present
7368dc9 
Fix: use direct comparison instead of strings.Contains to filter layernames

chore: fix inaccessible secrets

chore: fix invalid mapping
@iwpnd iwpnd force-pushed the ci/docker-build-only-with-secrets branch from aa8e390 to 7368dc9 Compare July 3, 2024 19:30
@coveralls
Copy link

coveralls commented Jul 3, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 26ed63dc0-PR-996

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.713%
Totals Coverage Status
Change from base Build c7296fdf0: 0.0%
Covered Lines: 6551
Relevant Lines: 14024
💛 - Coveralls

@ARolek ARolek merged commit f21dd1d into go-spatial:master Jul 3, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ARolek ARolek ARolek approved these changes

@gdey gdey Awaiting requested review from gdey gdey is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iwpnd @coveralls @ARolek