Error when parsing repository that uses multiselect custom-properties

[spalberg]

Problem overview

We started using githubs custom properties recently and noticed that our reviewdog/action-actionlint checks started to fail with the following error:

reviewdog: failed to build repo base HTML URL: json: cannot unmarshal array into Go struct field Repository.custom_properties of type string

reviewdog/action-actionlint uses reviewdog/reviewdog which in turn uses the current version of google/go-github to parse github related json.

Analysis

Handling for custom_properties was added in this PR: #3065.
The field is expected to be a map of string values (https://github.com/google/go-github/pull/3065/files#diff-7a0544e44d81e972917728b13efd3fa55e7f9d1bd8edb745216494fa1e178f93R80).

The actual api response can also include non-string values. The following is part of an actual api response obtained by using the github cli (gh api repos/OWNER/REPO):

{
  // ...
  "allow_update_branch": true,
  "use_squash_pr_title_as_default": false,
  "squash_merge_commit_message": "COMMIT_MESSAGES",
  "squash_merge_commit_title": "COMMIT_OR_PR_TITLE",
  "merge_commit_message": "PR_TITLE",
  "merge_commit_title": "MERGE_MESSAGE",
  "custom_properties": {
    "org-managed-rulesets": "true",
    "technologies": [
      "helm",
      "java"
    ]
  },
  // ...
}

The custom property technologies is multi select and github seems to return the selected values as an array which causes the parser to error out. The custom property org-managed-rulesets is of type boolean and represented using a string by github.

Sadly, githubs api docs are really bad when it comes to custom_property, so I can't say for sure if there are other output options than string or array of strings:

{
  // ...
  "custom_properties": {
      "type": "object",
      "description": "The custom properties that were defined for the repository. The keys are the custom property names, and the values are the corresponding custom property values.",
      "additionalProperties": true
  }
  // ...
}

[gmlewis]

Oh! Wow... TIL! Thank you for the report, @spalberg !

So to truly support anything, it sounds like it needs to be changed to a json.RawMessage, correct?
Then every client can parse it in a manner that makes sense to them.

Thoughts?

[gmlewis]

That would be a breaking API change, but if you agree, would you like to put together a PR, @spalberg ?

[spalberg]

Thanks for the quick response @gmlewis!

Disclaimer: I have yet to write my first line of go 😅

But I think that map[string]interface{} would be a good type for CustomProperties as it should be the literal translation of "type": "object" + "additionalProperties": true from githubs openapi spec. What do you think?

I could also try myself on that PR over the weekend.

[gmlewis]

Sounds like a great plan, @spalberg !

Welcome to Go! It is one of my all-time favorite programming languages and I think you will enjoy it!
The tooling is superb.

Great... the issue is yours. We have some helpful tips in the CONTRIBUTING.md file to help you get started.
(And if anything is missing there, feel free to contribute to that file as well. 😂 )

[spalberg]

Hi @gmlewis,
I opened a pr that widens the type as discussed and adds a few tests. Open for any feedback 😄