Repository navigation
gh api -F <key=val> not accepting int or boolean on ruleset creation #8983
Description
Activity
I see the same behaviour on
v2.48.0andv2.47.0but I wonder if this is a platform bug.Looking at your examples using the
--verboseflag, the body looks correct to me:{ "enforcement": "active", "name": "Test", "rules": [ { "parameters": { "required_approving_review_count": 1 }, "type": "pull_request" } ], "target": "branch" }{ "enforcement": "active", "name": "Test", "rules": [ { "parameters": { "dismiss_stale_reviews_on_push": true }, "type": "pull_request" } ], "target": "branch" }Is there anything that looks wrong in these request to you? I must admit I'm not an expert on the ruleset API. Also, the example
ghsnippet in those docs is definitely broken in some ways.@AlexVermette-Eaton : Just to share how I was testing #8762 by creating rule:
QUERY=' mutation($input:CreateRepositoryRulesetInput!){ createRepositoryRuleset(input:$input){ ruleset{ id name createdAt } } } ' GH_DEBUG=api ./bin/gh api graphql -f query="$QUERY" \ -F 'input[name]=test repo rule name3' \ -F 'input[enforcement]=DISABLED' \ -F 'input[conditions][refName][include][]=~DEFAULT_BRANCH' \ -F 'input[conditions][refName][exclude][]=refs/heads/ignore-*' \ -F 'input[sourceId]=R_kgDOIVGotg' \ -F 'input[rules][][type]=CREATION'
Let me be clear, this took a bit of reversing by manually creating a rule and playing with it.
Reacted by Alex VermetteIs there anything that looks wrong in these request to you?
The requests look good to me. This is exactly what i would expect. To get the latest structure of the ruleset object i used the export tool of rulesets available in the GitHub UI and looked at the json file it gave.
Also, the example gh snippet in those docs is definitely broken in some ways.
It definitely is broken. I should have probably reported this too but here's what I found that is broken:
- The
ref_name[include]is not valid, what needs to be used is something likeconditions[ref_name][include] - same goes for
ref_name[exclude]
I would assume
bypass_actors[][actor_id]=234is also broken for the same reason as my 2 examples above but I have not tested this particular key in my use cases.- The
If anyone has a work around I would gladly take it, as this is blocking for what
I am trying to achieve.I left a note for the team that owns this API endpoint on Saturday but given that it was the weekend I wouldn't expect a reply. In terms of your original comment, the way to have the most control is to craft your own JSON body and go from there:
{ "query": "mutation($input:CreateRepositoryRulesetInput!){\n createRepositoryRuleset(input:$input){\n ruleset{\n id\n name\n createdAt\n }\n }\n}", "variables": { "input": { "enforcement": "ACTIVE", "name": "Test", "conditions": {}, "rules": [ { "parameters": { "pullRequest": { "dismissStaleReviewsOnPush": false, "requireCodeOwnerReview": false, "requireLastPushApproval": false, "requiredApprovingReviewCount": 1, "requiredReviewThreadResolution": false } }, "type": "PULL_REQUEST" } ], "sourceId": "<REPOSITORY_GQL_ID>", "target": "BRANCH" } } }gh api graphql --verbose --input <FILE_CONTAINING_JSON_ABOVE>You can get the
<REPOSITORY_GQL_ID>via:gh repo list <OWNER> --json "id,name"I appreciate this is not particularly pleasant. I will get back to you when we hear back from the team that own the ruleset REST API.
Reacted by Alex VermetteLooks like the issue is that for the
parametersof arule, all fields are required as in the docs:
What this means is that you need a body like so:
{ "enforcement": "active", "name": "Test", "rules": [ { "parameters": { "dismiss_stale_reviews_on_push": false, "require_code_owner_review": false, "require_last_push_approval": false, "required_approving_review_count": 1, "required_review_thread_resolution": false }, "type": "pull_request" } ], "target": "branch" }Could be used like so:
gh api --verbose -X POST \ --input <FILE_CONTAINING_JSON_ABOVE> \ repos/{owner}/{repo}/rulesetsThe
-Fflags for fields are mostly suited for simple cases and can get janky very quickly with the syntax that was chosen. I'm not sure there's a way to represent this JSON structure via-F, so you may end up having to do some preprocessing to produce the JSON yourself (it can be passed on stdin with--input -. Have asked @andyfeller to see if he can massage-Fbecause he knows things 🤓 . Does it matter to you whether-Fworks for this case?Limitation of #8761 involving nested objects within arrays
@williammartin : I believe we're experiencing a limitation behind
parseFieldsdesign as it unclear when to create new nested objects over multiple fields within an array over multiple field flags.Explanation
The following explanation resolves around the following 2 blocks of code:
Lines 63 to 84 in 8181c62
Lines 135 to 150 in 8181c62
Take the following example:
gh api -X POST "repos/{owner}/{repo}/rulesets" \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ -f "name=Test" \ -f "target=branch" \ -f "enforcement=active" \ -F "rules[][type]=pull_request" \ -F "rules[][parameters][dismiss_stale_reviews_on_push]=false" \ -F "rules[][parameters][require_code_owner_review]=false" \ -F "rules[][parameters][require_last_push_approval]=false" \ -F "rules[][parameters][required_approving_review_count]=1" \ -F "rules[][parameters][required_review_thread_resolution]=false"
The problem occurs because
parseFieldsand related logic doesn't know when to start a new object within an array. It parses field keys one step at a time without taking later keys into account before deciding if this is setting an existing object or a new object.Assuming the following state before
rulesfield is being parsed:{ "name": "Test", "target": "branch", "enforcement": "active", }parseFieldsgenerates a stack of keys to set field values while detecting when an object is nested in an array:-
-F "rules[][type]=pull_request"parseFieldsseesrulesas the previous key andtypeas the new key and is aware it is in an array but not how deep the array is. Becausetestdoesn't exist within the map,addParamsSlicecreates a new map and appends it to the array before being returned to set the value resulting in:{ "name": "Test", "target": "branch", "enforcement": "active", "rules": [ { "type": "pull_request" } ] } -
-F "rules[][parameters][dismiss_stale_reviews_on_push]=false"Again,
parseFieldsseesrulesas the previous key andparametersas the new key and is aware it is in an array. Becauseparametersdoesn't exist within the map,addParamsSlicecreates a new map and appends it to the array before being returned to set the value resulting in:{ "name": "Test", "target": "branch", "enforcement": "active", "rules": [ { "type": "pull_request", "parameters": { "dismiss_stale_reviews_on_push": false } } ] } -
-F "rules[][parameters][require_code_owner_review]=false"This is where we get into the problem:
parseFieldsseesrulesandparametersjust like before.addParamsSliceseesparametersDOES exist however it doesn't fall into any existing use case to be reused as it doesn't have enough information to know we will set a totally different field than the last time.{ "name": "Test", "target": "branch", "enforcement": "active", "rules": [ { "type": "pull_request", "parameters": { "dismiss_stale_reviews_on_push": false } }, { "parameters": { "require_code_owner_review": false } } ] }
This feels like a problem with the field set design as users cannot set a whole object in a single flag.
Test for debugging
Locally, I created the following test within
pkg/cmd/api/fields_test.go:func Test_parseFields_nested2(t *testing.T) { ios, stdin, _, _ := iostreams.Test() fmt.Fprint(stdin, "pasted contents") opts := ApiOptions{ IO: ios, RawFields: []string{ "name=Test", "target=branch", "enforcement=active", "rules[][type]=pull_request", }, MagicFields: []string{ "rules[][parameters][dismiss_stale_reviews_on_push]=false", "rules[][parameters][require_code_owner_review]=false", "rules[][parameters][require_last_push_approval]=false", "rules[][parameters][required_approving_review_count]=1", "rules[][parameters][required_review_thread_resolution]=false", }, } params, err := parseFields(&opts) if err != nil { t.Fatalf("parseFields error: %v", err) } jsonData, err := json.MarshalIndent(params, "", "\t") if err != nil { t.Fatal(err) } assert.Equal(t, strings.TrimSuffix(heredoc.Doc(`{}`), "\n"), string(jsonData)) }
-
The -F flags for fields are mostly suited for simple cases and can get janky very quickly with the syntax that was chosen. I'm not sure there's a way to represent this JSON structure via -F, so you may end up having to do some preprocessing to produce the JSON yourself (it can be passed on stdin with --input -. Have asked @andyfeller to see if he can massage -F because he knows things 🤓 . Does it matter to you whether -F works for this case?
Your suggested approach is answering all my needs. I don't specifically needed the
-Fflag to work, I just needed a way to pass the whole ruleset data to the creation API and what you suggested is 100% doing the job for me; I just tested it. You can either close this issue or leave it open if you think this still needs to be fixed. I would suggest maybe reworking the API documentation before closing this.Reacted by Andy FellerI tripped up over this bug, trying to ( ultimately set up a ruleset as described above ). I noticed that the examples given in the API documentation didn't work either. I'm currently using
gh version 2.49.2 (2024-05-13)on Ubuntu 23.10The workaround worked perfectly for me to create a ruleset
cat << EOF | gh api --method POST -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28" --input - /repos/${ORG}/${REPO}/rulesets { "enforcement": "active", "name": "Require PullRequest", "rules": [ { "parameters": { "dismiss_stale_reviews_on_push": false, "require_code_owner_review": false, "require_last_push_approval": false, "required_approving_review_count": 1, "required_review_thread_resolution": false }, "type": "pull_request" } ], "bypass_actors": [ { "actor_id": 1234, "actor_type": "Team", "bypass_mode": "always" } ], "current_user_can_bypass": "always", "conditions": { "ref_name": { "include": [ "~DEFAULT_BRANCH" ], "exclude": [ ] } }, "target": "branch" } EOFand I learnt a lot about the --input - flag, as well as the --verbose flag.
I can't help but wonder if we should include in the gh API manual page an example of how to use the --input - flag to correctly pass the json in when all else fails. Also add a --verbose example too, as this provides useful debugging information when it comes to finding out what is going wrong.
I created a zendesk ticket 2777197 for myself, in the hope that if a customer logs a support ticket, then support will search and find the ticket - which will point them here.
Reacted by David Pérez-SuárezI can't help but wonder if we should include in the gh API manual page an example of how to use the --input - flag to correctly pass the json in when all else fails. Also add a --verbose example too, as this provides useful debugging information when it comes to finding out what is going wrong.
@erichanson1 : I think that is an excellent idea especially because how you do it is likely to differ by person. For example, my
andyfeller/gh-repo-exportextension builds dynamic JSON inputs usingjqwhich is a bit more advanced but possibly necessary depending:local REPOSITORIES=("$@") # shellcheck disable=SC2016 local MIGRATION_JQ='{ "lock_repositories": $lock_repositories, "exclude_attachments": $exclude_attachments, "exclude_git_data": $exclude_git_data, "exclude_metadata": $exclude_metadata, "exclude_owner_projects": $exclude_owner_projects, "exclude_releases": $exclude_releases, "repositories": $ARGS.positional, }' local MIGRATION_INPUTS=$( jq -n "$MIGRATION_JQ" \ --argjson exclude_attachments "$EXCLUDE_ATTACHMENTS" \ --argjson exclude_git_data "$EXCLUDE_GIT_DATA" \ --argjson exclude_metadata "$EXCLUDE_METADATA" \ --argjson exclude_owner_projects "$EXCLUDE_OWNER_PROJECTS" \ --argjson exclude_releases "$EXCLUDE_RELEASES" \ --argjson lock_repositories "$LOCK_REPOSITORIES" \ --args "${REPOSITORIES[@]}") local MIGRATION_ID=$(echo "$MIGRATION_INPUTS" | gh api "/orgs/$ORGANIZATION/migrations" -X POST -p wyandotte --input - --jq '.id')
Created #9104 to track this example addition and have marked it
help wantedso if you're interested in adding an example that would have helped you @erichanson1, that would be much appreciated. Cheers.I'm going to close this because #9104 tracks the action item for this work. Cheers everyone for your input!
Describe the bug
Version : gh version 2.48.0
Using the gh api command to create ruleset, it is impossible to pass anything other than a string as a "key=value" pair. I tried using
-Fflag but it does not work either.Here's an example:
gh api -X POST \ -H "Accept: application/vnd.github+json"\ -H "X-GitHub-Api-Version: 2022-11-28"\ repos/$owner/$repo/rulesets\ -f "name=Test"\ -f "target=branch"\ -f "enforcement=active"\ -f "rules[][type]=pull_request"\ -F "rules[][parameters][required_approving_review_count]=1"I get the following error:
{ "message": "Invalid request.\n\nInvalid property /rules/0: data matches no possible input. See `documentation_url`.", "documentation_url": "https://docs.github.com/rest/repos/rules#create-a-repository-ruleset" }I get the same behavior when i try with a property that should have a boolean value. For example:
gh api -X POST \ -H "Accept: application/vnd.github+json"\ -H "X-GitHub-Api-Version: 2022-11-28"\ repos/$owner/$repo/rulesets\ -f "name=Test"\ -f "target=branch"\ -f "enforcement=active"\ -f "rules[][type]=pull_request"\ -F "rules[][parameters][dismiss_stale_reviews_on_push]=true"