-
Notifications
You must be signed in to change notification settings - Fork 23
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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(`\/{.*(=.*)}`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small comment on this: because |
||
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} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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)} There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Version 1 (as-is implementation)This is essentially "feature parity" with the existing process. It relies on the
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 2This pulls in the "server version" that is being used to generate the client and overrides the 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 https://github.com/psprings/client-go/blob/swagger-compatibility/generate/main.go#L46-L54
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 3If Version 2 was agreeable enough, it seems less meaningful to have a
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 All of that to say, I guess this could be manually supplied for now via the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree that specifying the client version via Happy to merge this once the client version change is in. It sounds like we're on the same page about everything else! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulling in both swagger specs to have access to the |
||
} | ||
|
||
func swaggerGenerate(mergedClient bool) { | ||
swaggerSpecNames := getSwaggerNames() | ||
for _, swaggerSpecName := range swaggerSpecNames { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} |
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 thego 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
By adding in the
project.swagger.json
we now also get access to: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?There was a problem hiding this comment.
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:
project.swagger.json
spec in the client generation (at a minimum)[and/or]
Presumably the
Configuration
type will always be the same underlying struct [literal] between thegrafeasAPI
andgrafeasProjectAPI
above.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 commongrafeasAPI
client?There was a problem hiding this comment.
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.