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

Fix import of external private repos and add private repo test #597

Merged
merged 6 commits into from
May 13, 2021

Conversation

christophebedard
Copy link
Member

@christophebedard christophebedard commented Mar 24, 2021

Closes #270

This adds a test that uses import-token along with a repos file with a private repo.

It also switches to a different approach for using the import-token globally. This should hopefully fix all remaining private repo issues.

Supersedes #592 (see comments)

Signed-off-by: Christophe Bedard bedard.christophe@gmail.com

@christophebedard christophebedard requested a review from a team as a code owner March 24, 2021 21:14
@christophebedard christophebedard requested review from Karsten1987 and jaisontj and removed request for a team March 24, 2021 21:14
required-ros-distributions: foxy
- uses: ./
with:
import-token: ${{ secrets.TOKEN_PRIVATE_REPO_TEST }}
Copy link
Contributor

Choose a reason for hiding this comment

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

i think you may be able to just use GITHUB_TOKEN here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try but from what I can tell it doesn't work.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I don't think it works. From what I can tell GITHUB_TOKEN is for the current repo only.

Copy link
Contributor

Choose a reason for hiding this comment

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

@emersonknapp @christophebedard Hello, thank you for working on this feature!

I've forked and tested this in kenji-miyake#2.
As @christophebedard says, GITHUB_TOKEN is for the current repo and couldn't be overridden.
So I generated a personal token from here and added it as secrets.ACTION_ROS_CI_TOKEN from here.

image

After that, CI seems to succeed.
image

In ros-tooling org, I believe a personal token can be replaced with organization secrets or something else.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kenji-miyake thanks for confirming!

Since this PR actually contains both a test and a fix for private repos, we should get this merged!

@emersonknapp could you look into creating an organization secret and adding it to this repo as TOKEN_PRIVATE_REPO_TEST? I can't do it (since I'm not an owner I think).

Copy link
Contributor

Choose a reason for hiding this comment

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

Organization Secrets are just a way to set a Secret at the level of the Organization, which individual repositories can then use. The contents of the secret itself are still a personal access token. I don't see a need for this particular token to be used outside this test - so I've added it just to this repo. It has "repo" permissions only - so it should be able to check out private repositories.

I updated the branch which is restarting the workflows - we'll see what happens when the queue clears out. Thanks for bumping that dependabot config down to weekly - what a hassle this backed up queue is. I'm still investigating more ways to thin it out even further.

Copy link
Member Author

Choose a reason for hiding this comment

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

the test passed! 🎊

but now the branch is out of date 😆 is there any way to override that setting and squash+rebase anyway?

@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #597 (f8cadab) into master (bc8e741) will decrease coverage by 0.67%.
The diff coverage is 0.00%.

❗ Current head f8cadab differs from pull request most recent head c268b9e. Consider uploading reports for the commit c268b9e to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #597      +/-   ##
==========================================
- Coverage   39.20%   38.52%   -0.68%     
==========================================
  Files           2        2              
  Lines         227      231       +4     
  Branches       44       46       +2     
==========================================
  Hits           89       89              
- Misses        138      142       +4     
Impacted Files Coverage Δ
src/action-ros-ci.ts 29.70% <0.00%> (-0.61%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc8e741...c268b9e. Read the comment docs.

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
This reverts commit 16180de.

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
@christophebedard christophebedard changed the title Add test with repos file containing a private repo Fix import of external private repos and add private repo test May 9, 2021
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.

Create ros-tooling private repo to test this build with access tokens
3 participants