Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion .github/workflows/linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ jobs:
go-version: stable
cache-dependency-path: "**/go.sum"
- name: Check OpenAPI
run: ./script/metadata.sh update-openapi --validate
run: |
./script/metadata.sh update-openapi --validate
./script/metadata.sh check-schema-fields
env:
CHECK_GITHUB_OPENAPI: 1
GITHUB_TOKEN: ${{ github.token }}
33 changes: 33 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,39 @@ Its subcommands are:
- `unused` - lists operations from `openapi_operations.yaml` that are not mapped
from any methods.

- `check-schema-fields` - automatically matches GitHub's OpenAPI component
schemas to Go request structs when the JSON field set makes the match
unambiguous, then reports JSON field optionality mismatches. Ambiguous or
unsupported schemas are skipped instead of configured with per-schema
exceptions. It can be used to check whether required, non-nullable schema
fields are represented as non-pointer fields without `omitempty` or
`omitzero`, and whether optional schema fields remain omittable in Go. For
example:

```sh
script/metadata.sh check-schema-fields
```

To experiment with one schema while refactoring, pass `--schema` with the
OpenAPI schema name. Filtered schemas also allow high-confidence schema-name
matches and response structs so the command can report the current
differences before the JSON field set is fully aligned:

```sh
script/metadata.sh check-schema-fields --schema repository-ruleset --verbose
```

Use `--include-responses` to inspect response structs in bulk. This is useful
for measuring drift, but response required fields are treated more cautiously
than request bodies in this project.

A few Go fields intentionally deviate from the OpenAPI schema (for example a
required field kept as a pointer pending a value-parameter refactor). These are
listed as `Struct.Field` entries in
`tools/metadata/schema_field_exceptions.yaml`, and their diagnostics are
suppressed; each is a known deviation to fix and remove over time. Update that
file (rather than the Go source) to add or remove an exception.

[OpenAPI descriptions of their API]: https://github.com/github/rest-api-description

## Scripts
Expand Down
12 changes: 10 additions & 2 deletions script/lint.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/sh
#/ [ CHECK_GITHUB_OPENAPI=1 ] script/lint.sh runs linters and validates generated files.
#/ When CHECK_GITHUB is set, it validates that openapi_operations.yaml is consistent with the
#/ descriptions from github.com/github/rest-api-description.
#/ When CHECK_GITHUB_OPENAPI is set, it validates OpenAPI metadata and schema fields
#/ against descriptions from github.com/github/rest-api-description.

set -e

Expand Down Expand Up @@ -96,6 +96,14 @@ if [ -n "$CHECK_GITHUB_OPENAPI" ]; then
printf "${RED}✘ openapi_operations.yaml validation failed${NC}\n"
fail
fi

print_header "Validating OpenAPI schema fields"
if script/metadata.sh check-schema-fields; then
printf "${GREEN}✔ OpenAPI schema fields are valid${NC}\n"
else
printf "${RED}✘ OpenAPI schema field validation failed${NC}\n"
fail
fi
fi

print_header "Validating generated files"
Expand Down
77 changes: 75 additions & 2 deletions tools/metadata/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,18 @@ Update go source code to be consistent with openapi_operations.yaml.

"format_help": `Format white space in openapi_operations.yaml and sort its operations.`,
"unused_help": `List operations in openapi_operations.yaml that aren't used by any service methods.`,
"check_schema_fields_help": `
Check Go struct JSON field optionality against GitHub's OpenAPI schemas. By default, the check automatically
matches OpenAPI component schemas to Go request structs only when the JSON field set makes the match unambiguous.
Use --schema to try one or more OpenAPI schema names; filtered schemas also allow high-confidence schema-name
matches and response structs to make refactoring experiments easier.
`,

"working_dir_help": `Working directory. Should be the root of the go-github repository.`,
"openapi_ref_help": `Git ref to pull OpenAPI descriptions from.`,
"working_dir_help": `Working directory. Should be the root of the go-github repository.`,
"openapi_ref_help": `Git ref to pull OpenAPI descriptions from.`,
"openapi_ref_default_help": `Git ref to pull OpenAPI descriptions from. Defaults to openapi_commit from openapi_operations.yaml.`,
"schema_filter_help": `OpenAPI schema name to check. May be repeated. Defaults to all automatically matched schemas.`,
"include_responses_help": `Also check response structs. By default only request structs are checked unless --schema is provided.`,

"openapi_validate_help": `
Instead of updating, make sure that the operations in openapi_operations.yaml's "openapi_operations" field are
Expand All @@ -54,6 +63,7 @@ type rootCmd struct {
UpdateGo updateGoCmd `kong:"cmd,help=${update_go_help}"`
Format formatCmd `kong:"cmd,help=${format_help}"`
Unused unusedCmd `kong:"cmd,help=${unused_help}"`
CheckSchema checkSchemaCmd `kong:"cmd,name=check-schema-fields,help=${check_schema_fields_help}"`

WorkingDir string `kong:"short=C,default=.,help=${working_dir_help}"`

Expand Down Expand Up @@ -182,6 +192,69 @@ func (c *unusedCmd) Run(root *rootCmd, k *kong.Context) error {
return nil
}

type checkSchemaCmd struct {
Ref string `kong:"help=${openapi_ref_default_help}"`
Schemas []string `kong:"name=schema,help=${schema_filter_help}"`
IncludeResponses bool `kong:"name=include-responses,help=${include_responses_help}"`
Verbose bool `kong:"help='Print checked and skipped schema matches.'"`
}

func (c *checkSchemaCmd) Run(root *rootCmd, k *kong.Context) error {
ctx := context.Background()
_, opsFile, err := root.opsFile()
if err != nil {
return err
}
ref := c.Ref
if ref == "" {
ref = opsFile.GitCommit
if ref == "" {
return errors.New("openapi_operations.yaml does not have an openapi_commit field")
}
}

client, err := githubClient(root.GithubURL, root.UploadURL)
if err != nil {
return err
}
descriptions, err := getDescriptions(ctx, client, ref)
if err != nil {
return err
}
exceptions, err := loadSchemaFieldExceptions(root.WorkingDir)
if err != nil {
return err
}
result, err := checkSchemaFields(schemaFieldCheckOptions{
descriptions: descriptions,
githubDir: filepath.Join(root.WorkingDir, "github"),
schemaNames: c.Schemas,
includeResponses: c.IncludeResponses,
exceptions: exceptions,
})
if err != nil {
return err
}

fmt.Fprintf(k.Stdout, "Found %v schema field issues\n", len(result.Diagnostics))
fmt.Fprintf(k.Stdout, "Checked %v OpenAPI schema/Go struct pairs; skipped %v OpenAPI schemas\n", result.Summary.Checked, result.Summary.Skipped)
if c.Verbose {
for _, checked := range result.Checked {
fmt.Fprintf(k.Stdout, "checked: %v -> %v (%v)\n", checked.OpenAPISchema, checked.GoStruct, checked.MatchReason)
}
for _, skipped := range result.Skipped {
fmt.Fprintf(k.Stdout, "skipped: %v (%v)\n", skipped.OpenAPISchema, skipped.Reason)
}
}
for _, diag := range result.Diagnostics {
fmt.Fprintln(k.Stdout, diag.String())
}
if len(result.Diagnostics) > 0 {
return fmt.Errorf("found %v schema field issues", len(result.Diagnostics))
}
return nil
}

func main() {
err := run(os.Args[1:], nil)
if err != nil {
Expand Down
15 changes: 15 additions & 0 deletions tools/metadata/schema_field_exceptions.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# The file lists "Struct.Field" entries whose JSON field optionality intentionally deviates from the OpenAPI schema,
# so that their check-schema-fields diagnostics are suppressed.
#
# TODO: fix these fields and remove the exceptions.
exceptions:
- DependencyGraphSnapshot.Detector
- DependencyGraphSnapshot.Job
- DependencyGraphSnapshot.Ref
- DependencyGraphSnapshot.Scanned
- DependencyGraphSnapshot.Sha
- DeploymentBranchPolicyRequest.Name
- ReviewCustomDeploymentProtectionRuleRequest.Comment
- WorkflowsPermissionsOpt.RequireApprovalForForkPRWorkflows
- WorkflowsPermissionsOpt.SendSecretsAndVariables
- WorkflowsPermissionsOpt.SendWriteTokensToWorkflows
Loading
Loading