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

Adding istio.io/api to the CUE unity corpus #2055

Closed
myitcv opened this issue Jul 15, 2021 · 1 comment
Closed

Adding istio.io/api to the CUE unity corpus #2055

myitcv opened this issue Jul 15, 2021 · 1 comment

Comments

@myitcv
Copy link

myitcv commented Jul 15, 2021

In istio/tools#1539, @jasonwzm and @howardjohn kindly helped to get an upgrade to cuelang.org/go@v0.4.0 merged, a change that was then automatically picked up in this repository via aadf1ba (AFAICT). Furthermore, as mentioned in istio/tools#1539 (comment), I will soon submit a follow-up PR that removes the //nolint:staticcheck annotations so that cue-gen uses non-deprecated CUE API.

Istio is a key CUE user, especially when it comes to the encoding/openapi package. As such, we would like to add Istio to the unity test corpus. unity is a tool used to run experiments/regression tests on various CUE modules using different versions of CUE. The repository that contains the unity tool is also a corpus of CUE modules against which unity is run, and we would like to add https://github.com/istio/api to that corpus.

To the best of my understanding, in order to do that we need to understand:

  1. The version of https://github.com/istio/tools used for a given commit of https://github.com/istio/api (which implies the version of cuelang.org/go being used).
  2. What commands to run to replicate the generation process.
  3. The version of Go in operation at any given time.

Regarding each point.

Point 1

My understanding is that dependency is described here:

export IMAGE_VERSION=master-2021-05-25T23-11-11

Ideally we would like to have that dependency described in terms of a go.mod file. But I can understand that adding istio.io/tools to the the istio.io/api module is undesirable (because it would add a significant number of minimum version constraints to the module graphs of any users of istio.io/api).

Is there perhaps a way we can code generate a go.mod file in a subdirectory that describes the dependency? e.g.

-- _tools/go.mod --
module tools

go 1.16

require istio.io/tools v0.0.0-20210526005057-047b0c7489d1
-- _tools/tools.go --
//go:build tools
// +build tools

package tools

import _ "istio.io/tools/cmd/cue-gen"

Point 2

My understanding is that the current steps are fully described by these two lines:

api/gen.sh

Lines 43 to 44 in ec23a4c

cue-gen -paths=common-protos -f=./cue.yaml
cue-gen -paths=common-protos -f=./cue.yaml --crd=true -snake=jwksUri,apiKeys,apiSpecs,includedPaths,jwtHeaders,triggerRules,excludedPaths,mirrorPercent

Assuming this is the case, the only question would be how we keep this script in sync with any tests that look to replicate those two steps (a comment at each location might suffice).

Point 3

We need to understand the Go version(s) used to run istio.io/tools/cmd/cue-gen within the https://github.com/istio/api repo. The cuelang.org/go module version will be answered by point 1; the Go version(s) will then complete the definition of the "base" case for our testing.

How would the tests work?

At least for now I simply envisage:

  • re-running the two generation steps using the "base" case versions of cuelang.org/go and Go to confirm git status --porcelain is clean
  • running the two generation steps for different versions of cuelang.org/go (i.e. varying the CUE version), and again chekcing git status --porcelain is clean

This would give us an early warning signal in case any of the output changes (as well as a basic indication of performance changes).


@jasonwzm and @howardjohn - I would very much welcome your thoughts on the above. If a call to discuss would be a better way to start this discussion we'd be happy to run such a session.

cc @mpvl

@howardjohn
Copy link
Member

We are no longer using CUE so I am not sure this is particular useful anymore. LMK if not

@howardjohn howardjohn closed this as not planned Won't fix, can't repro, duplicate, stale May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants