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

Initial Swagger compatibility proposal #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
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
205 changes: 205 additions & 0 deletions generate/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
package main

import (
"bufio"
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"log"
"net/http"
"os"
"os/exec"
"path"
"regexp"
"strings"
)

// ServerVersion : the version from the upstream repository
var ServerVersion = getDefaultVersion()

// GrafeasRepository : the repository of the Grafeas server to use for downloading Swagger files
var GrafeasRepository = "https://github.com/grafeas/grafeas"

// Reference : the commit, branch, or tag to use for downloading Swagger files from the defined Grafeas repo
var Reference = "master"

// APIVersion : the version of the Grafeas API per the Google Cloud scheme - https://github.com/grafeas/grafeas/blob/master/docs/versioning.md#api
var APIVersion = "v1beta1"

// SwaggerCodegenVersion : the version of the Swagger CodeGen CLI to download
var SwaggerCodegenVersion = "2.4.5"

// MergedClient : whether to keep the generated Swagger clients separate or to merge the paths
// TODO: implement function that merges the paths of each Swagger spec into one file before running codegen
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I follow this: what would the merge look like of each Swagger spec? What's the end result?

Copy link
Author

Choose a reason for hiding this comment

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

TL;DR I took the liberty of pulling in the project API spec, and thought it might be nice to merge it with the grafeas spec at some point.

I hadn't taken the time to implement it yet, but the idea was (I'm sure you can speak to the validity of this more than I can) was to have a more uniform "client" experience. Right now, the code generation was only using grafeas.swagger.json from the upstream grafeas repository. Because of this, it is missing some of the project API interactions (e.g. create, delete, list, get). I think that this would be nice to have as part of the overarching client. So, at a minimum I have pulled in the projects.swagger.json from the upstream repo as part of the go generate
Define all swagger spec files
https://github.com/grafeas/client-go/pull/15/files#r379532306
Iterate over all swagger spec files
https://github.com/grafeas/client-go/pull/15/files#r379532718

With that in place the interaction looks something like

package main

import (
	"context"
	"fmt"
	"log"

	grafeasAPI "github.com/psprings/client-go/0.1.4"
)

func main() {
	projectID := "projects/myproject"
	grafeasCfg := &grafeasAPI.Configuration{
		BasePath:      "http://localhost:8080",
		DefaultHeader: make(map[string]string),
		UserAgent:     "Swagger-Codegen/0.1.0/go",
	}
	grafeasClient := grafeasAPI.NewAPIClient(grafeasCfg)

	notesResp, _, err := grafeasClient.GrafeasV1Beta1Api.ListNotes(context.Background(), projectID, nil)
	if err != nil {
		log.Fatal(err)
	} else {
		log.Print(notesResp.Notes)
	}
}

By adding in the project.swagger.json we now also get access to:

package main

import (
	"context"
	"fmt"
	"log"

	grafeasProjectAPI "github.com/psprings/client-go/0.1.4/project"
)

func main() {
	grafeasProjectCfg := &grafeasProjectAPI.Configuration{
		BasePath:      "http://localhost:8080",
		DefaultHeader: make(map[string]string),
		UserAgent:     "Swagger-Codegen/0.1.0/go",
	}
	grafeasProjectClient := grafeasProjectAPI.NewAPIClient(grafeasProjectCfg)
	ctx := context.Background()
	projectID := "projects/myproject"
	createdProject, _, err := grafeasProjectClient.ProjectsApi.CreateProject(ctx, grafeasProjectAPI.ProjectProject{
		Name: projectID,
	})
	if err != nil {
		log.Println(err)
	}
	log.Println(createdProject)
	projectsResp, _, _ := grafeasProjectClient.ProjectsApi.ListProjects(ctx, nil)
	log.Printf("%#v", projectsResp)
}

My thought was that it might be a more desirable/uniform experience to merge the spec files in such a way that you only have to do one client import. If I'm off base here, I can remove the project.swagger.json import and scrap this logic?

Copy link
Author

@psprings psprings Mar 9, 2020

Choose a reason for hiding this comment

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

Hey @aysylu what do you think about adding the project API to the generated client (to accompany the generated "grafeas" API) per my comment above?

Any thoughts on the notion of:

  1. Including the project.swagger.json spec in the client generation (at a minimum)

[and/or]

  1. Merging the two swagger spec files to form a cohesive client capable with interacting with both APIs?

Presumably the Configuration type will always be the same underlying struct [literal] between the grafeasAPI and grafeasProjectAPI above.

// Grafeas API Configuration & Client
grafeasCfg := &grafeasAPI.Configuration{
    BasePath:      "http://localhost:8080",
}
grafeasClient := grafeasAPI.NewAPIClient(grafeasCfg)
// Project Configuration & Client
grafeasProjectCfg := &grafeasProjectAPI.Configuration{
    BasePath:      "http://localhost:8080",
}
grafeasProjectClient := grafeasProjectAPI.NewAPIClient(grafeasProjectCfg)

Instead of having to do conversion or reflection or just duplicating the Configuration to interact with both parts of the API, it seems like it might be more desirable to the average consumer of the client library to interact with a common grafeasAPI client?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @psprings, thanks for clarifying! I absolutely agree that merging the 2 APIs for the more powerful client is a great idea. And it's a strict improvement over the previous state.

var MergedClient = false

// trimVersionTag : remove the "v" prefix from the version tag string
func trimVersionTag(tag string) string {
if strings.HasPrefix(tag, "v") {
return strings.TrimPrefix(tag, "v")
}
return tag
}

func getDefaultVersion() string {
version := os.Getenv("VERSION")
if version == "" {
tag := getMostRecentTag()
trimmedVersion := trimVersionTag(tag)
version = trimmedVersion
}
return version
}

func getMostRecentTag() string {
tagsBytes, _ := get("https://api.github.com/repos/grafeas/grafeas/tags")
var tags []map[string]interface{}
json.Unmarshal(tagsBytes, &tags)
return tags[0]["name"].(string)
}

func swaggerCompatibility(jsonInput string) string {
re := regexp.MustCompile(`\/{.*(=.*)}`)

Choose a reason for hiding this comment

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

Small comment on this: because GetNote: "/v1beta1/{name=projects/*/notes/*}" and GetOccurrence: "/v1beta1/{name=projects/*/occurrences/*}" both share the same form of /v1beta1/{name}, the swagger generation only renders GetOccurrence.
I don't think there's a way of fixing this other than changing the grafeas.swagger.json.

jsonOutput := re.ReplaceAllStringFunc(jsonInput, func(substringMatch string) string {
return strings.Split(substringMatch, "=")[0] + "}"
})
return jsonOutput
}

func get(url string) ([]byte, error) {
var responseData []byte
resp, err := http.Get(url)
if err != nil {
return responseData, err
}
defer resp.Body.Close()

responseData, err = ioutil.ReadAll(resp.Body)
if err != nil {
return responseData, err
}
return responseData, err
}

func downloadCompatibleSwaggerSpec(url string, filename string) error {
if filename == "" {
filename = path.Base(url)
}
responseBytes, err := get(url)
if err != nil {
return err
}

swaggerString := swaggerCompatibility(string(responseBytes))

f, err := os.Create(filename)
if err != nil {
return err
}
defer f.Close()
responseReader := bytes.NewReader([]byte(swaggerString))

_, err = io.Copy(f, responseReader)
return err
}

func downloadFile(url string, filename string) error {
if filename == "" {
filename = path.Base(url)
}
responseBytes, err := get(url)
if err != nil {
return err
}

f, err := os.Create(filename)
if err != nil {
return err
}
defer f.Close()
responseReader := bytes.NewReader(responseBytes)

_, err = io.Copy(f, responseReader)
return err
}

type swaggerCodegenConfig struct {
language string
inputSpec string
outputDirectory string
configFile string
jar string
}

func stringOrDefault(currentValue string, defaultValue string) string {
if currentValue == "" {
return defaultValue
}
return currentValue
}

func (c *swaggerCodegenConfig) generate() {
c.language = stringOrDefault(c.language, "go")
c.jar = stringOrDefault(c.jar, "./swagger-codegen-cli.jar")
c.configFile = stringOrDefault(c.configFile, "./config.go.json")
c.outputDirectory = stringOrDefault(c.outputDirectory, ServerVersion)
args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-c", c.configFile}
Copy link
Author

Choose a reason for hiding this comment

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

Could use this logic instead of updating and committing the version in the config.json each time:

args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-c", c.configFile, fmt.Sprintf("-DpackageVersion=", ServerVersion)}

Or something like this to forego the file entirely

args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-DpackageName=grafeas", fmt.Sprintf("-DpackageVersion=", ServerVersion)}

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify: what would be the difference in output in the 3 versions discussed here?

Copy link
Author

@psprings psprings Mar 18, 2020

Choose a reason for hiding this comment

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

Ultimately, the output in terms of directory name and content would be the same. From my perspective this just streamlines the process a bit, ensures a little more consistency (ensures packageVersion matches output directory override), and is a little more CI server friendly? Hopefully this helps explain it a little bit better.

Version 1 (as-is implementation)

This is essentially "feature parity" with the existing process. It relies on the config.go.json being updated by the user prior to running the client generate.

  • config.go.json provides both the packageVersion and the packageName in this example
  • command line args remain "unmodified" from the existing implementation
args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-c", c.configFile}

Which would yield something like:

java -jar ./swagger-codegen-cli.jar generate -i ./grafeas.swagger.json -l go -o 0.1.0 -c ./config.go.json

Version 2

This pulls in the "server version" that is being used to generate the client and overrides the packageVersion present in the config.go.json (or means, in essence, that you can omit this value from that file altogether). As a result, there is more of an assurance that the version is consistent, and could be more automation friendly.

The nice thing about this is that version can be passed in as one of the flags, or can be ingested via an environment variable. If neither are specified, this will grab the latest tag from the grafeas/grafeas repository and "release" that.

https://github.com/psprings/client-go/blob/swagger-compatibility/generate/main.go#L46-L54

  • config.go.json provides only the packageName
  • the command line args provide packageVersion
args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-c", c.configFile, fmt.Sprintf("-DpackageVersion=", ServerVersion)}

Which would yield something like:

java -jar ./swagger-codegen-cli.jar generate -i ./grafeas.swagger.json -l go -o 0.1.0 -c ./config.go.json -DpackageVersion=0.1.0

Version 3

If Version 2 was agreeable enough, it seems less meaningful to have a config.go.json file which only provides a single value (packageName) so you might as well just leverage that as a command line arg.

  • config.go.json provides nothing (and can be removed)
  • the command line args provide both packageName and packageVersion
args := []string{"-jar", c.jar, "generate", "-i", c.inputSpec, "-l", c.language, "-o", c.outputDirectory, "-DpackageName=grafeas", fmt.Sprintf("-DpackageVersion=", ServerVersion)}

Which would yield something like:

java -jar ./swagger-codegen-cli.jar generate -i ./grafeas.swagger.json -l go -o 0.1.0 -c ./config.go.json -DpackageName=grafeas -DpackageVersion=0.1.0

Copy link
Contributor

@aysylu aysylu Mar 18, 2020

Choose a reason for hiding this comment

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

Got it, thanks so much for the thorough clarifications on this! I agree that more automation-friendly approach would be better, and I like Version 2/3 because this allows for more automated generation of values. Moving packageName out of the configuration seems reasonable. For packageVersion, I'm open to automating this as well, with one detail: the intention here is to separate server release version from the client, eg server is at version 0.1.4 and the client is 0.2.1 because of some new features and bug fixes added to the client, which are independent of the server releases. What'd you suggest as the best way to address this, while minimizing the use of config.go.json?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, okay that makes sense. I was operating under the assumption that there was some coordination of the client version with the server version. We could pretty easily include a ClientVersion in the code which is accessible via ldflags when doing generation which could be supplied as a flag or otherwise passed as an environment variable (CLIENT_VERSION or GRAFEAS_CLIENT_VERSION etc) which would be ingested and would determine the output directory name and the packageVersion flag?

I was trying to avoid needing to vendor any additional (external) dependencies so, I don't have anything doing semantic version parsing or incrementing, but the logic could be something like

if len(ClientVersion) == 0 {
  // I've used something like https://github.com/Masterminds/semver in the past
  // for some of the parsing and incrementing functionality
  ClientVersion = autoIncrementPatch()
}

But this would rely on version being known at which point you would need a VERSION, version.go, or I suppose the config.go.json file unless you do some logic to extrapolate tag based on current commit (assuming that this repo would be subject to tags and releases).

All of that to say, I guess this could be manually supplied for now via the config.go.json, via a ldflag and/or environment variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree that specifying the client version via config.go.json, via ldflag and env var would be great here, as you suggested above. My concern with auto-incrementing versions is that it'd be harder to encode whether micro, minor or major version should change based on the commits, but this is a decision humans can make more effectively.

Happy to merge this once the client version change is in. It sounds like we're on the same page about everything else!

Copy link
Author

Choose a reason for hiding this comment

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

Great! I will try to set aside a little bit of time in the next day or two to get that sorted and committed.

log.Println("[CMD] java " + strings.Join(args, " "))
cmd := exec.Command("java", args...)

stderr, _ := cmd.StderrPipe()
cmd.Start()

scanner := bufio.NewScanner(stderr)
scanner.Split(bufio.ScanLines)
for scanner.Scan() {
m := scanner.Text()
fmt.Println(m)
}
cmd.Wait()
}

func downloadSwaggerCodegenCli(version string) {
downloadURL := fmt.Sprintf("https://repo1.maven.org/maven2/io/swagger/swagger-codegen-cli/%s/swagger-codegen-cli-%s.jar", version, version)
downloadFile(downloadURL, "swagger-codegen-cli.jar")
}

func getSwaggerNames() []string {
return []string{"grafeas.swagger.json", "project.swagger.json"}
Copy link
Author

Choose a reason for hiding this comment

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

Pulling in both swagger specs to have access to the ProjectsAPI in addition to the GrafeasV1Beta1Api

}

func swaggerGenerate(mergedClient bool) {
swaggerSpecNames := getSwaggerNames()
for _, swaggerSpecName := range swaggerSpecNames {
Copy link
Author

Choose a reason for hiding this comment

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

Iterate over all defined swagger specs to generate all API interactions

swaggerSpecURL := strings.Join([]string{GrafeasRepository, "/raw/", Reference, "/proto/", APIVersion, "/swagger/", swaggerSpecName}, "")
log.Printf("[DOWNLOAD] %s\n", swaggerSpecURL)
downloadCompatibleSwaggerSpec(swaggerSpecURL, "")
fileBasename := strings.Split(swaggerSpecName, ".")[0]
outputDirectory := strings.Join([]string{ServerVersion, fileBasename}, "/")
if fileBasename == "grafeas" && !mergedClient {
outputDirectory = ServerVersion
}
swaggerCodeGen := swaggerCodegenConfig{
inputSpec: swaggerSpecName,
outputDirectory: outputDirectory,
}
swaggerCodeGen.generate()
}
}

func main() {
log.Printf(
`

aysylu marked this conversation as resolved.
Show resolved Hide resolved
Grafeas Repository: %s
Reference: %s
Server Version: %s
API Version: %s
Swagger Codegen Version: %s

`, GrafeasRepository, Reference, ServerVersion, APIVersion, SwaggerCodegenVersion)
downloadSwaggerCodegenCli(SwaggerCodegenVersion)
swaggerGenerate(MergedClient)
}
4 changes: 1 addition & 3 deletions generate_client.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Downloads v1beta1 grafeas.swagger.json, swagger-codegen CLI tool,
// and generates Go client library from the downloaded Swagger spec.

//go:generate wget https://github.com/grafeas/grafeas/raw/master/proto/v1beta1/swagger/grafeas.swagger.json
//go:generate wget https://repo1.maven.org/maven2/io/swagger/swagger-codegen-cli/2.4.5/swagger-codegen-cli-2.4.5.jar -O swagger-codegen-cli.jar
//go:generate java -jar ./swagger-codegen-cli.jar generate -i ./grafeas.swagger.json -l go -o 0.1.0 -c ./config.go.json
//go:generate go run generate/main.go

package generate_client