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

Deploy container images for both main and PRs #69

Closed
wants to merge 3 commits into from

Conversation

chambridge
Copy link
Contributor

  • Add script to set the QUAY_IMAGE_TAG based on main or pull request
  • Update travis push-to-quay stage

Related to: https://pulp.plan.io/issues/5175

* Add script to set the QUAY_IMAGE_TAG based on main or pull request
* Update travis push-to-quay stage

Related to: https://pulp.plan.io/issues/5175
fixes #5175
@pulpbot
Copy link
Member

pulpbot commented Feb 9, 2021

Attached issue: https://pulp.plan.io/issues/5175

@fao89
Copy link
Member

fao89 commented Feb 9, 2021

@chambridge we are using github actions now, sorry about that, we need to remove travis files to avoid confusion

@chambridge
Copy link
Contributor Author

Looks like the pull request process is all GitHub Actions 🤷 and not travis-ci (must be leftover) . I'll circle up with @mikedep333 on just extending .github/workflows/pr.yml.

@chambridge
Copy link
Contributor Author

@fao89 Thanks must have been typing at the same time 😄 I can clean up the travis files if you like as a part of this.

@fao89
Copy link
Member

fao89 commented Feb 9, 2021

@fao89 Thanks must have been typing at the same time I can clean up the travis files if you like as a part of this.

it would be great!

Add quay push PR stage.
Create latest image for ci test and tag image tag for pr.
Comment on lines +239 to +242
- name: Setting secrets
run: python3 .github/workflows/scripts/secrets.py "$SECRETS_CONTEXT"
env:
SECRETS_CONTEXT: ${{ toJson(secrets) }}
Copy link
Member

@fao89 fao89 Feb 10, 2021

Choose a reason for hiding this comment

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

actually github actions don't expose secrets on PRs.

In order to protect public repositories for malicious users 
we run all pull request workflows raised from repository forks 
with a read-only token and no access to secrets.

https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Makes sense, but seems like if we want PR images pushed into quay we either need travis-ci or a Jenkins to be able to do this.

Copy link
Member

Choose a reason for hiding this comment

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

we migrated from travis because of their new open-source policy

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I do not know how to overcome this security limitation.

Other than to possibly create a different quay.io account for PRs.

Copy link
Member

Choose a reason for hiding this comment

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

I strongly object. The security limitation is there for a reason. We should not even try to overcome it.
Is the "local" registry (not doing any push) not sufficient to serve any pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine by me. Do we want to update https://pulp.plan.io/issues/5175 to just be travis clean up or close it out with master being built?

@chambridge
Copy link
Contributor Author

Closing this out PR and associated 5175. Will open a new task for the travis clean up based on my conversation with @mikedep333

@chambridge chambridge closed this Feb 17, 2021
@chambridge chambridge deleted the 5175-push-prs-to-quay branch February 17, 2021 19:55
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

Successfully merging this pull request may close these issues.

None yet

5 participants