Skip to content

Conversation

@retu2libc
Copy link
Contributor

@retu2libc retu2libc commented Jun 8, 2023

Closes #26 -- Adds support for:

$ art configure $GITLAB_URL --token-type [private|job] $TOKEN

and maintains legacy support for:

$ art configure $GITLAB_URL $TOKEN

@haboustak
Copy link
Collaborator

Hey @christheyankee, thanks for the PR.

Overall I think this looks good, however we should only store 1 token of any type in config.yml. It's unfortunate that the current key name is private_token, rather than just token. Our goal should be for the config schema to mirror the command-line.

backwards-compatible:

private_token: 'o_bla98df'

or modern:

token: 'o_bla98df'
token_type: 'private'

We would support loading the old format, but only save the modern format.

If there are multiple tokens in the configuration, we don't know which token to use, and trying to use both tokens would create even more unpredictable behavior than already provided by the inconsistent permissions available via CI_JOB_TOKEN. When the user runs art configure TOKEN they expect art to use that token, replacing any previously defined value.

@retu2libc
Copy link
Contributor Author

When the user runs art configure TOKEN they expect art to use that token, replacing any previously defined value.

This behavior would actually be preserved since that would store a user token and python-gitlab uses the user token over the CI token when provided both.

@retu2libc retu2libc requested a review from haboustak June 9, 2023 03:43
@haboustak
Copy link
Collaborator

python-gitlab uses the user token over the CI token when provided both.

python-gitlab does do that currently, but it's just a hack they must implement because they pull tokens from myriad sources.

Their order of precedence ensures there aren't errors from the GitLab request, but it doesn't make any claim that the correct token is used.

art has more reasonable configuration. You tell it what token to use, and it uses that token, because only 1 token can be used. (unless/until we add logic to retry API requests that fail authentication)

@retu2libc
Copy link
Contributor Author

ping @haboustak since you might not have seen f33b2b8

@haboustak
Copy link
Collaborator

Hey @christheyankee, sorry for the delay in getting back to you - I've been traveling.

When I get a chance over the next few days I'll pull down your version and use some of my existing projects to test and ensure the conversions work as expected. Should be good to go though.

@haboustak
Copy link
Collaborator

I finally managed to get around to trying this out over the weekend, sorry for taking so long.

The configuration and backward-compatibility changes all worked. Unfortunately, I couldn't get art download to download artifacts with a job token without further changes, and I don't think that art update can be made to work.

When testing art download using a job token, I received a 404 Not Found response when accessing the project GitLab API endpoint, for example: GET /api/v4/projects/open-source%2Fmusl-cross-make.

According to the CI_JOB_TOKEN docs a job token cannot be used to access the project or jobs API endpoints, only job artifacts. To work around this limitation, we can change the download command to get a lazy reference to the project (see this commit)

That should fix download, but the update command needs API access to both pipelines and jobs. It's probably OK for some use cases to support download differently than update, as long as we print a very clear error message when update is called and the token-type is job.

I performed my testing so far using GitLab CE, although I don't think EE would change the results.

retu2libc and others added 4 commits September 16, 2023 08:44
The CI_JOB_TOKEN cannot be used to access the projects API endpoint.
Using a shallow project object allows us to access the ProjectJobsManager
without making an API request.
The job token doesn't provide access to the API endpoints required to
perform the update command. If you try it anyway, GitLab returns a 404
status code when accessing a project via the API using a job token. Work
around this by checking the type of token during update and print a
reasonable error message.
retu2libc and others added 5 commits September 16, 2023 11:34
Fix token availability typo
ClickExceptions are handled by click and result in an error message
without a stack trace. This commit also provides a config-specific
Exception base class to provide additional context as to the source of
configuration errors.

Example:

    Error: config.token_type: A job token cannot be used to update artifacts
Handle the case where config has both the old and new token settings.
Also improve error reporting when config is missing (entirely, or any
required element).
@haboustak
Copy link
Collaborator

Thanks @christheyankee!

@haboustak haboustak merged commit d87dbd7 into kosma:master Sep 18, 2023
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.

Add CI_JOB_TOKEN support

3 participants