gh api -F <key=val> not accepting int or boolean on ruleset creation

[AlexVermette-Eaton]

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 -F flag 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"

[williammartin]

I see the same behaviour on v2.48.0 and v2.47.0 but I wonder if this is a platform bug.

Looking at your examples using the --verbose flag, 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 gh snippet in those docs is definitely broken in some ways.

[andyfeller]

@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.

[AlexVermette-Eaton]

@williammartin

Is 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 like conditions[ref_name][include]
• same goes for ref_name[exclude]

I would assume bypass_actors[][actor_id]=234 is also broken for the same reason as my 2 examples above but I have not tested this particular key in my use cases.

[AlexVermette-Eaton]

If anyone has a work around I would gladly take it, as this is blocking for what
I am trying to achieve.

[williammartin]

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.

[williammartin]

Looks like the issue is that for the parameters of a rule, 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}/rulesets

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?

[andyfeller]

Limitation of #8761 involving nested objects within arrays

@williammartin : I believe we're experiencing a limitation behind parseFields design 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:

cli/pkg/cmd/api/fields.go

Lines 63 to 84 in 8181c62

	destMap := params
	isArray := false
	var subkey string
	for _, k := range keystack {
	if k == "" {
	isArray = true
	continue
	}
	if subkey != "" {
	var err error
	if isArray {
	destMap, err = addParamsSlice(destMap, subkey, k)
	isArray = false
	} else {
	destMap, err = addParamsMap(destMap, subkey)
	}
	if err != nil {
	return err
	}
	}
	subkey = k

cli/pkg/cmd/api/fields.go

Lines 135 to 150 in 8181c62

	func addParamsSlice(m map[string]interface{}, prevkey, newkey string) (map[string]interface{}, error) {
	if v, exists := m[prevkey]; exists {
	if existSlice, ok := v.([]interface{}); ok {
	if len(existSlice) > 0 {
	lastItem := existSlice[len(existSlice)-1]
	if lastMap, ok := lastItem.(map[string]interface{}); ok {
	if _, keyExists := lastMap[newkey]; !keyExists {
	return lastMap, nil
	} else if reflect.TypeOf(lastMap[newkey]).Kind() == reflect.Slice {
	return lastMap, nil
	}
	}
	}
	newMap := make(map[string]interface{})
	m[prevkey] = append(existSlice, newMap)
	return newMap, nil

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 parseFields and 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 rules field is being parsed:

{
    "name": "Test",
    "target": "branch",
    "enforcement": "active",
}

parseFields generates a stack of keys to set field values while detecting when an object is nested in an array:

1. -F "rules[][type]=pull_request"

   parseFields sees rules as the previous key and type as the new key and is aware it is in an array but not how deep the array is. Because test doesn't exist within the map, addParamsSlice creates 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"
           }
       ]
   }

2. -F "rules[][parameters][dismiss_stale_reviews_on_push]=false"

   Again, parseFields sees rules as the previous key and parameters as the new key and is aware it is in an array. Because parameters doesn't exist within the map, addParamsSlice creates 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
               }
           }
       ]
   }

3. -F "rules[][parameters][require_code_owner_review]=false"

   This is where we get into the problem: parseFields sees rules and parameters just like before. addParamsSlice sees parameters DOES 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))
}

[AlexVermette-Eaton]

@williammartin

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 -F flag 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.

[erichanson1]

@williammartin @andyfeller

I 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.10

The 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"
}
EOF

and 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.

[andyfeller]

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.

@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-export extension builds dynamic JSON inputs using jq which 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')