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

Fix authorization method #97

Closed
wants to merge 2 commits into from
+1 −4
Closed

Fix authorization method #97

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd56183
Fix authorization method
leonardbinet Feb 11, 2022
150d7b1
Use 'Circle-Token' header for authentication
leonardbinet Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/commands/wait_for_job.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,17 +46,14 @@ steps:
exit 0;
fi

api_endpoint="api/v2/workflow/${CIRCLE_WORKFLOW_ID}/job"

mkdir -p /tmp/swissknife

# This is a global variable used to get return value for get_job_status
job_status=""
job_number=""

get_job_status() {
wf_url="https://circleci.com/$api_endpoint?circle-token=${CIRCLE_TOKEN}"
curl -f -s $wf_url > /tmp/swissknife/wf_$CIRCLE_WORKFLOW_ID.json
curl --header 'authorization: Basic ${CIRCLE_TOKEN}' -f -s https://circleci.com/api/v2/workflow/${CIRCLE_WORKFLOW_ID}/job > /tmp/swissknife/wf_$CIRCLE_WORKFLOW_ID.json
Copy link
Owner

Choose a reason for hiding this comment

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

if we are changing this you need to use

Circle-Token

as the header name not basic i believe.

also this orb is used in production and works right now. interesting to see that you get not found 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the proposed method in documention here https://circleci.com/docs/api/v2/#operation/listWorkflowJobs
Screenshot 2022-02-11 at 22 38 58

I'll try again next week and let you know, could it be linked somehow to using a different version of pipelines?
I was using version: 2.1.

Copy link
Owner

Choose a reason for hiding this comment

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

ah yeah the circleci docs arent great at the auth part. this PR will break the orb :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry my message wasn't very clear, based on documentation there are three possible authentication methods:

  • api_key_header using Circle-Token header key
  • basic_auth using basic authentication (the one I implemented in this PR)
  • api_key_query using circle-token key in query parameters (not in headers), which is the method your are currently using, and which will be deprecated at some point

Screenshot 2022-02-12 at 22 59 47

I used basic_auth but api_key_header would work fine as well :)

Note: before making this PR I launched a CircleCi build with SSH session to test it, and it works for me, so I don't think it would break the orb (or is there something I missed 🤔 ?):
Screenshot 2022-02-12 at 22 59 16

For some reason, in my case the api_key_query method doesn't work though.
Screenshot 2022-02-12 at 22 58 43

This error wasn't covered and resulted in a 22 error code:
Screenshot 2022-02-12 at 22 56 15
I investigated via SSH to understand where the issue came from, and to me it seems related to authentication method.

By the way, thanks a lot for making this orb, I don't understand how this feature isn't built-in in circleci, your orb is really appreciated :)

Cheers!

Copy link
Contributor Author

@leonardbinet leonardbinet Feb 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

note: I just noticed this issue #76 is related to a exit code 22 as well, it might be the same underlying issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, there might be another issue.
When running the current script in ssh mode, circleci api authentication fails, resulting in job_status and job_number variables having "" value.
Which is equivalent to "the job we wait for doesn't exist", since we check "$job_status" == "":

if [[ "$job_status" == "success" || "$job_status" == "failed" || "$job_status" == "canceled" || "$job_status" == "" ]]; then
    echo "Its finally my turn. exiting"

This is an issue that will be solved by proper authentication, but I cannot reproduce the code 22 error though.

Copy link
Contributor Author

@leonardbinet leonardbinet Feb 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think I found the reason for the code 22 exit status:
https://curl.se/mail/archive-2003-03/0079.html

--fail will return 22 for any HTTP error that is 400 or above

For some reason the -s option in curl command hid it, but it seems to be the culprit.
Screenshot 2022-02-13 at 20 41 34

Because circleci orbs run with #!/bin/bash -eo pipefail, even if script ends with a exit 0, the status will be the last failing.
https://stackoverflow.com/questions/68465355/what-is-the-meaning-of-set-o-pipefail-in-bash-script
Screenshot 2022-02-13 at 20 54 30

Now, I've no idea why it occurs on some configuration, but not on yours (could you check if authentication really works, or if the failing auth is just silently ignored, resulting in thinking the job we wait for doesn't exist?)

Copy link
Owner

Choose a reason for hiding this comment

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

so i double checked this. the PR in the current state will not work.

=> Authorization header does not work with circle token.
=> to use the Circle token method you need to have header Circle-Token

i.e.

does not work

curl --location --request GET 'https://circleci.com/api/v2/pipeline?org-slug=gh/roopakv' \
--header 'Authorization: Basic TOKEN_HERE'

works

curl --location --request GET 'https://circleci.com/api/v2/pipeline?org-slug=gh/roopakv' \
--header 'Circle-Token: TOKEN_HERE'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any preference so let's go with the Circle-Token header 👍

It's strange though, for me it works in both cases 🤷‍♂️
Screenshot 2022-02-14 at 11 57 14

job_status=$(jq -r '.items[] | select(.name=="<< parameters.job-name >>") | .status' /tmp/swissknife/wf_$CIRCLE_WORKFLOW_ID.json)
job_number=$(jq -r '.items[] | select(.name=="<< parameters.job-name >>") | .job_number' /tmp/swissknife/wf_$CIRCLE_WORKFLOW_ID.json)
}
Expand Down