Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenAPI V3 Root interface and basic implementation #115393

Merged
merged 1 commit into from Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
168 changes: 168 additions & 0 deletions staging/src/k8s.io/client-go/openapi3/root.go
@@ -0,0 +1,168 @@
/*
Copyright 2023 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package openapi3

import (
"encoding/json"
"fmt"
"sort"
"strings"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/openapi"
"k8s.io/kube-openapi/pkg/spec3"
)

// Root interface defines functions implemented against the root
// OpenAPI V3 document. The root OpenAPI V3 document maps the
// API Server relative url for all GroupVersions to the relative
// url for the OpenAPI relative url. Example for single GroupVersion
// apps/v1:
//
// "apis/apps/v1": {
// "ServerRelativeURL": "/openapi/v3/apis/apps/v1?hash=<HASH>"
// }
type Root interface {
seans3 marked this conversation as resolved.
Show resolved Hide resolved
// GroupVersions returns every GroupVersion for which there is an
// OpenAPI V3 GroupVersion document. Returns an error for problems
// retrieving or parsing the OpenAPI V3 root document.
GroupVersions() ([]schema.GroupVersion, error)
// GVSpec returns the specification for all the resources in a
// GroupVersion as a pointer to a spec3.OpenAPI struct.
// Returns an error for problems retrieving or parsing the root
// document or GroupVersion OpenAPI V3 document.
GVSpec(gv schema.GroupVersion) (*spec3.OpenAPI, error)
// GVSpecAsMap returns the specification for all the resources in a
// GroupVersion as unstructured bytes. Returns an error for
// problems retrieving or parsing the root or GroupVersion
// OpenAPI V3 document.
GVSpecAsMap(gv schema.GroupVersion) (map[string]interface{}, error)
}

// root implements the Root interface, and encapsulates the
// fields to retrieve, store the parsed OpenAPI V3 root document.
type root struct {
// OpenAPI client to retrieve the OpenAPI V3 documents.
client openapi.Client
apelisse marked this conversation as resolved.
Show resolved Hide resolved
}

// Validate root implements the Root interface.
var _ Root = &root{}

// NewRoot returns a structure implementing the Root interface,
// created with the passed rest client.
func NewRoot(client openapi.Client) Root {
return &root{client: client}
}

func (r *root) GroupVersions() ([]schema.GroupVersion, error) {
paths, err := r.client.Paths()
if err != nil {
return nil, err
}
// Example GroupVersion API path: "apis/apps/v1"
gvs := make([]schema.GroupVersion, 0, len(paths))
for gvAPIPath := range paths {
gv, err := pathToGroupVersion(gvAPIPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we've seen that in the other PR, there are some paths that don't map to group-version unfortunately, we should probably ignore these? Unfortunately, I also don't like just being lenient here, do we have a way to be more strict yet allowing the non-matching files @Jefftree ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the non-gv in the test NewFileClient will also help you actually test that this code does the right thing by the way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any use cases of requiring the OpenAPI for non group-version paths? If there aren't, I think we should ignore them and users can use Raw() if they need those paths to do their own post processing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think they should do Raw, they should just not use this abstraction (call path and get the bytes?). Hmm, that's not ideal, but we don't have use-cases for them anyway, so let's wait for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an api path (e.g. api/apps/v1) does not correctly translate to a GroupVersion, we're now ignoring with a continue. Please have a look, including the tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you want to see happen Jeff? Part of the problem is that this API does two things:

  1. It parses paths into GVs
  2. It parses the raw bytes into specs (either map[string]interface{}, or openapi3.Spec).

We could possibly (in another PR), split these two responsibilities, i.e. have:

type Parser interface {
    Specs(path string) (*spec3.OpenAPI, error)
    SpecsAsMap(path string) (map[string]interface{}, error)
}

func NewParser(openapi.Client) Parser {
   ...
}

// And then, the code from this PR
// But now it has 1 responsibility: parse the path into GVs
func NewRoot(parser Parser) Root {
	return &openapi3Root{client: client}
}

And now if you want to access a non-gv openapi, you can use the parser instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just trying to understand use cases for accessing a non-gv openapi, if there aren't any I feel like there's no strong reason to split the responsibilities and make it more complex for the user. I like that we're handling both the GV and raw bytes parsing in the same interface so users don't have to deal with the underlying abstraction.

if err != nil {
// Ignore paths which do not parse to GroupVersion
continue
}
gvs = append(gvs, gv)
}
// Sort GroupVersions alphabetically
sort.Slice(gvs, func(i, j int) bool {
return gvs[i].String() < gvs[j].String()
})
return gvs, nil
}

func (r *root) GVSpec(gv schema.GroupVersion) (*spec3.OpenAPI, error) {
openAPISchemaBytes, err := r.retrieveGVBytes(gv)
if err != nil {
return nil, err
}
// Unmarshal the downloaded Group/Version bytes into the spec3.OpenAPI struct.
var parsedV3Schema spec3.OpenAPI
err = json.Unmarshal(openAPISchemaBytes, &parsedV3Schema)
return &parsedV3Schema, err
}

func (r *root) GVSpecAsMap(gv schema.GroupVersion) (map[string]interface{}, error) {
gvOpenAPIBytes, err := r.retrieveGVBytes(gv)
if err != nil {
return nil, err
}
// GroupVersion bytes into unstructured map[string] -> empty interface.
var gvMap map[string]interface{}
err = json.Unmarshal(gvOpenAPIBytes, &gvMap)
return gvMap, err
}

// retrieveGVBytes returns the schema for a passed GroupVersion as an
// unstructured slice of bytes or an error if there is a problem downloading
// or if the passed GroupVersion is not supported.
func (r *root) retrieveGVBytes(gv schema.GroupVersion) ([]byte, error) {
paths, err := r.client.Paths()
if err != nil {
return nil, err
}
apiPath := gvToAPIPath(gv)
gvOpenAPI, found := paths[apiPath]
if !found {
return nil, fmt.Errorf("GroupVersion (%s) not found in OpenAPI V3 root document", gv)
}
return gvOpenAPI.Schema(runtime.ContentTypeJSON)
}

// gvToAPIPath maps the passed GroupVersion to a relative api
apelisse marked this conversation as resolved.
Show resolved Hide resolved
// server url. Example:
//
// GroupVersion{Group: "apps", Version: "v1"} -> "apis/apps/v1".
func gvToAPIPath(gv schema.GroupVersion) string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there such util function already implemented somewhere that can be reused directly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/rest/url_utils.go#L64-L78

Found an existing util function for groupversion -> path but not sure if there exists something for the other way around (path -> groupversion)

var resourcePath string
if len(gv.Group) == 0 {
resourcePath = fmt.Sprintf("api/%s", gv.Version)
} else {
resourcePath = fmt.Sprintf("apis/%s/%s", gv.Group, gv.Version)
}
return resourcePath
}

// pathToGroupVersion is a helper function parsing the passed relative
// url into a GroupVersion.
//
// Example: apis/apps/v1 -> GroupVersion{Group: "apps", Version: "v1"}
// Example: api/v1 -> GroupVersion{Group: "", Version: "v1"}
func pathToGroupVersion(path string) (schema.GroupVersion, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. it seems to be a common util function.

var gv schema.GroupVersion
parts := strings.Split(path, "/")
if len(parts) < 2 {
return gv, fmt.Errorf("Unable to parse api relative path: %s", path)
}
apiPrefix := parts[0]
if apiPrefix == "apis" {
gv.Group = parts[1]
gv.Version = parts[2]
Copy link
Member

@Jefftree Jefftree Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seans3 Line 155 enforces the length to be 2 or more, but this line accesses the third index in the array. This causes certain paths to fail when the length is exactly 2 with runtime error: index out of range [2] with length 2 (eg: /apis/apps)

Copy link
Contributor Author

@seans3 seans3 Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/apis/apps does not work because it is not an acceptable input. It should be apis/apps (without leading /). And it should always have a Version, so it should be apis/apps/v1.

Copy link
Contributor Author

@seans3 seans3 Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talk offline, I have a better idea of the issue--thanks.

} else if apiPrefix == "api" {
gv.Version = parts[1]
} else {
return gv, fmt.Errorf("Unable to parse api relative path: %s", path)
}
return gv, nil
}