octokit / go-sdk

A generated Go SDK from GitHub's OpenAPI specification. Built on Kiota.
MIT License
71 stars 8 forks source link

[BUG]: GET request for repos/OWNER/REPO/properties/values broken #88

Open felixlut opened 4 months ago

felixlut commented 4 months ago

What happened?

I'm playing around with this sdk as a means of contributing to https://github.com/integrations/terraform-provider-github/issues/1956, and as such my focus is on the endpoints related to repository custom properties.

The issue I'm facing occurs when calling octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Properties().Values().Get(), which is mapped to the repos/OWNER/REPO/properties/values GET endpoint. I'll list the code I'm using further down in the message.

When running the code on a repository where no custom properties are set, it works as intended by returning an empty list. But when using it on a repo where any custom property is set, the error below is the result. I've played around with properties of all available types (true_false, single_select, multi_select, and text), and all of them have this error. Error:

panic: index is empty.

For example, if I add a custom property called text with a value of test, I get the following response while using the gh cli (gh api repos/YesWeKanske/test-custom-properties/properties/values):

[
  {
    "property_name": "text",
    "value": "test"
  }
]

But I get the index is empty error with the code I've posted below.

I've ran a debugger and tracked this error down to this line in the microsoft/kiota-serialization-json-go library, which is being called from this line in pkg/github/models/custom_property_value.go. From what I can gather this part of the code is for converting the value (in this case the string "test") of a custom property to the structs the Kiota framework can parse (or something). Point is that it deals with the value of a custom property.

I'd be lying if I said I understand exactly what Kiota is doing under the hood, but the parseNode.GetChildNode("") looks weird to me. Calling that with an empty string will always render in an error, and there are quite a few examples throughout the codebase of this occurring. I've only tested it with the repository custom properties, but I'd assume the same errors would occur for any of the other resources as well.

My code:

package main

import (
    "context"
    "fmt"
    "log"
    "os"
    "time"

    abs "github.com/microsoft/kiota-abstractions-go"
    "github.com/octokit/go-sdk/pkg"
)

func main() {
    octokitClient, err := pkg.NewApiClient(
        pkg.WithUserAgent("my-user-agent"),
        pkg.WithRequestTimeout(5*time.Second),
        pkg.WithBaseUrl("https://api.github.com"),
        pkg.WithTokenAuthentication(os.Getenv("GITHUB_TOKEN")),
    )
    ctx := context.Background()

    if err != nil {
        log.Fatalf("error creating client: %v", err)
    }

    owner := "YesWeKanske"
    repoName := "test-custom-properties"

    defaultRequestConfig := &abs.RequestConfiguration[abs.DefaultQueryParameters]{
        QueryParameters: &abs.DefaultQueryParameters{},
    }
    repoURL := octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName)
    propURL := repoURL.Properties().Values()

    props, err := propURL.Get(ctx, defaultRequestConfig)
    if err != nil {
        panic(err)
    }
    fmt.Println(props)
}

Versions

I've used the latest version of the provider at the time of writing this bug report (go get github.com/octokit/go-sdk@a74a3a2a13654bd84794fd91a6a0fbc96f883722, where a74a3a2a13654bd84794fd91a6a0fbc96f883722 is a commit to this repo). My go.mod file looks like so:

module example/hello

go 1.22.3

require (
    github.com/microsoft/kiota-abstractions-go v1.6.0
    github.com/octokit/go-sdk v0.0.21-0.20240709170617-a74a3a2a1365
)

require (
    github.com/cjlapao/common-go v0.0.39 // indirect
    github.com/davecgh/go-spew v1.1.1 // indirect
    github.com/go-logr/logr v1.4.1 // indirect
    github.com/go-logr/stdr v1.2.2 // indirect
    github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
    github.com/google/uuid v1.6.0 // indirect
    github.com/kfcampbell/ghinstallation v0.0.6 // indirect
    github.com/microsoft/kiota-http-go v1.3.3 // indirect
    github.com/microsoft/kiota-serialization-form-go v1.0.0 // indirect
    github.com/microsoft/kiota-serialization-json-go v1.0.7 // indirect
    github.com/microsoft/kiota-serialization-multipart-go v1.0.0 // indirect
    github.com/microsoft/kiota-serialization-text-go v1.0.0 // indirect
    github.com/pmezard/go-difflib v1.0.0 // indirect
    github.com/std-uritemplate/std-uritemplate/go v0.0.55 // indirect
    github.com/stretchr/testify v1.9.0 // indirect
    go.opentelemetry.io/otel v1.24.0 // indirect
    go.opentelemetry.io/otel/metric v1.24.0 // indirect
    go.opentelemetry.io/otel/trace v1.24.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
)

Code of Conduct

github-actions[bot] commented 4 months ago

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

felixlut commented 4 months ago

I'll note that a workaround to this is fetching the repository directly via octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Get(), which parses the custom properties of the repo correctly. Looks something like:

repo, err := octokitClient.Repos().ByOwnerId(owner).ByRepoId(repoName).Get(...)
repoProps := repo.GetCustomProperties().GetAdditionalData()
colbylwilliams commented 3 months ago

Hit the same trying to get the current user:

error in user_request_builder

package dev

import (
    "context"
    "fmt"
    "os"

    octokit "github.com/octokit/go-sdk/pkg"
)

func main() {
    gh, err := octokit.NewApiClient(
        octokit.WithUserAgent("my-thing"),
        octokit.WithTokenAuthentication(os.Getenv("GITHUB_TOKEN")),
    )
    if err != nil {
        fmt.Println("error: ", err)
    }

    ctx := context.Background()

    user, err := gh.User().Get(ctx, nil)
    if err != nil {
        fmt.Println("error: ", err) // panic: index is empty
    }

    fmt.Println("user: ", user)
}
HowHow06 commented 1 month ago

Bumping into the same issue with client.Repos().ByOwnerId(owner).ByRepoId(repo).Rulesets().ByRuleset_id(rulesetId).Get(), which is mapped to /repos/{owner}/{repo}/rulesets/{ruleset_id}. The parseNode.GetChildNode("") causes an error of index is empty at this line as well.

// err == index is empty
response, err := client.Repos().ByOwnerId(owner).ByRepoId(repo).Rulesets().ByRuleset_id(rulesetId).Get(context.background(), nil)