From 8976e96b3cdd10a10d76fa241726db87edfe4231 Mon Sep 17 00:00:00 2001 From: Wen Bo Li <50884368+wenovus@users.noreply.github.com> Date: Tue, 9 Jan 2024 21:02:53 -0800 Subject: [PATCH] NOSImageProfile Validation for ocrpcs (#2529) * Fix nosimage.textproto rpcs examples * Fix lint * Validate ocrpcs * debug * Use HTTPS * Retrigger CI * fix * test don't expect failure * revert to good * Improve readability * Enforce that examples are re-generated * Fix staticcheck * Run at 00:49 to avoid peak * gofmt --- .github/workflows/nosimage.yml | 41 +++++++ go.mod | 1 + go.sum | 2 + tools/internal/ocrpcs/ocrpcs.go | 104 ++++++++++++++++++ tools/internal/ocrpcs/visitor.go | 94 ++++++++++++++++ tools/nosimage/README.md | 4 +- .../example_nosimageprofile_invalid.textproto | 30 +++-- tools/nosimage/example/generate_example.go | 29 +++-- tools/nosimage/validate/validate.go | 25 ++++- 9 files changed, 292 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/nosimage.yml create mode 100644 tools/internal/ocrpcs/ocrpcs.go create mode 100644 tools/internal/ocrpcs/visitor.go diff --git a/.github/workflows/nosimage.yml b/.github/workflows/nosimage.yml new file mode 100644 index 00000000000..73038133b49 --- /dev/null +++ b/.github/workflows/nosimage.yml @@ -0,0 +1,41 @@ +name: NOSImage validation script + +on: + push: + branches: [ main ] + pull_request: + schedule: + - cron: "49 0 * * *" + +jobs: + integration-test: + runs-on: ubuntu-latest + steps: + - name: Check out code + uses: actions/checkout@v3 + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: stable + cache: false + - name: Generate Examples and Check No Diff + run: | + cd tools/nosimage + go run example/generate_example.go -file-path example/example_nosimageprofile.textproto + go run example/generate_example.go -file-path example/example_nosimageprofile_invalid.textproto -invalid + + git diff --exit-code --ignore-all-space --ignore-blank-lines + + - name: Validate Good Example + run: | + cd tools/nosimage + go run validate/validate.go -file example/example_nosimageprofile.textproto; rm -rf tmp + + - name: Validate Bad Example + run: | + cd tools/nosimage + if go run validate/validate.go -file example/example_nosimageprofile_invalid.textproto; then + echo "Validation passed, but failure expected" + exit 1 + fi + rm -rf tmp diff --git a/go.mod b/go.mod index f34841ea59b..4ae4ea468f1 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/openconfig/ygot v0.29.17 github.com/p4lang/p4runtime v1.4.0-rc.5.0.20220728214547-13f0d02a521e github.com/protocolbuffers/txtpbfmt v0.0.0-20220608084003-fc78c767cd6a + github.com/yoheimuta/go-protoparser/v4 v4.9.0 golang.org/x/crypto v0.17.0 golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc golang.org/x/text v0.14.0 diff --git a/go.sum b/go.sum index 28e09b8781a..ace8d194b0f 100644 --- a/go.sum +++ b/go.sum @@ -1343,6 +1343,8 @@ github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= +github.com/yoheimuta/go-protoparser/v4 v4.9.0 h1:zHRXzRjkOamwMkPu7bpiCtOpxHkM9c8zxQOvW99eWlo= +github.com/yoheimuta/go-protoparser/v4 v4.9.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/tools/internal/ocrpcs/ocrpcs.go b/tools/internal/ocrpcs/ocrpcs.go new file mode 100644 index 00000000000..b948c9778a0 --- /dev/null +++ b/tools/internal/ocrpcs/ocrpcs.go @@ -0,0 +1,104 @@ +// Package ocrpcs contains utilities related to ocrpcs.proto. +package ocrpcs + +import ( + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/yoheimuta/go-protoparser/v4" + + rpb "github.com/openconfig/featureprofiles/proto/ocrpcs_go_proto" + "github.com/openconfig/gnmi/errlist" +) + +func cloneAPIRepo(downloadPath, api string) (string, error) { + if downloadPath == "" { + return "", fmt.Errorf("must provide download path") + } + repoPath := filepath.Join(downloadPath, api) + + cmd := exec.Command("git", "clone", "--single-branch", "--depth", "1", fmt.Sprintf("https://github.com/openconfig/%s.git", api), repoPath) + stderr, err := cmd.StderrPipe() + if err != nil { + return "", err + } + if err := cmd.Start(); err != nil { + return "", fmt.Errorf("failed to clone %s repo: %v, command failed to start: %q", api, err, cmd.String()) + } + stderrOutput, _ := io.ReadAll(stderr) + if err := cmd.Wait(); err != nil { + return "", fmt.Errorf("failed to clone %s repo: %v, command failed during execution: %q\n%s", api, err, cmd.String(), stderrOutput) + } + return repoPath, nil +} + +func readAllRPCs(downloadPath, api string) (map[string]struct{}, error) { + repoPath, err := cloneAPIRepo(downloadPath, api) + if err != nil { + return nil, err + } + + rpcs := map[string]struct{}{} + + if err := filepath.Walk(repoPath, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if strings.HasSuffix(info.Name(), ".proto") { + reader, err := os.Open(path) + if err != nil { + return err + } + got, err := protoparser.Parse(reader) + if err != nil { + return fmt.Errorf("failed to parse, err %v", err) + } + visitor := &rpcServiceAccumulator{} + got.Accept(visitor) + for _, rpc := range visitor.rpcs { + rpcs[rpc] = struct{}{} + } + } + return nil + }); err != nil { + return nil, err + } + + return rpcs, nil +} + +// ValidateRPCs verifies whether the RPCs listed in protocols are valid. +// +// It returns the number of valid RPCs found, and an error if any RPC is +// invalid, or there was an issue downloading or parsing OpenConfig protobuf +// files. +// +// - downloadPath is a path to the folder which will contain OpenConfig +// repositories that will be downloaded in order to validate the existence of +// provided RPCs. +func ValidateRPCs(downloadPath string, protocols map[string]*rpb.OCProtocol) (uint, error) { + var validCount uint + + var errs errlist.List + errs.Separator = "\n" + for api, protocol := range protocols { + rpcs, err := readAllRPCs(downloadPath, api) + if err != nil { + return 0, err + } + for _, name := range protocol.GetMethodName() { + if _, ok := rpcs[name]; !ok { + errs.Add(fmt.Errorf("RPC not found in openconfig repo %v: %v", api, name)) + continue + } + validCount++ + } + } + + return validCount, errs.Err() +} diff --git a/tools/internal/ocrpcs/visitor.go b/tools/internal/ocrpcs/visitor.go new file mode 100644 index 00000000000..6748224ce86 --- /dev/null +++ b/tools/internal/ocrpcs/visitor.go @@ -0,0 +1,94 @@ +package ocrpcs + +import ( + "fmt" + + "github.com/yoheimuta/go-protoparser/v4/parser" +) + +type rpcServiceAccumulator struct { + packageName string + currentService string + + rpcs []string +} + +var _ parser.Visitor = &rpcServiceAccumulator{} + +func (v *rpcServiceAccumulator) VisitComment(*parser.Comment) { +} + +func (v *rpcServiceAccumulator) VisitEmptyStatement(*parser.EmptyStatement) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitEnum(*parser.Enum) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitEnumField(*parser.EnumField) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitExtend(*parser.Extend) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitExtensions(*parser.Extensions) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitField(*parser.Field) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitGroupField(*parser.GroupField) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitImport(*parser.Import) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitMapField(*parser.MapField) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitMessage(*parser.Message) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitOneof(*parser.Oneof) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitOneofField(*parser.OneofField) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitOption(*parser.Option) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitPackage(p *parser.Package) (next bool) { + v.packageName = p.Name + return +} + +func (v *rpcServiceAccumulator) VisitReserved(*parser.Reserved) (next bool) { + return +} + +func (v *rpcServiceAccumulator) VisitRPC(r *parser.RPC) (next bool) { + v.rpcs = append(v.rpcs, fmt.Sprintf("%s.%s.%s", v.packageName, v.currentService, r.RPCName)) + return +} + +func (v *rpcServiceAccumulator) VisitService(s *parser.Service) (next bool) { + v.currentService = s.ServiceName + return true +} + +func (v *rpcServiceAccumulator) VisitSyntax(*parser.Syntax) (next bool) { + return +} diff --git a/tools/nosimage/README.md b/tools/nosimage/README.md index 1de7c505eaf..5f827a1c87d 100644 --- a/tools/nosimage/README.md +++ b/tools/nosimage/README.md @@ -4,12 +4,12 @@ ``` cd $GOPATH/src/github.com/openconfig/featureprofiles/tools/nosimage -go run validate/validate.go -file example/example_nosimageprofile.textproto; rm -rf public +go run validate/validate.go -file example/example_nosimageprofile.textproto; rm -rf tmp ``` ``` cd $GOPATH/src/github.com/openconfig/featureprofiles/tools/nosimage -go run validate/validate.go -file example/example_nosimageprofile_invalid.textproto; rm -rf public +go run validate/validate.go -file example/example_nosimageprofile_invalid.textproto; rm -rf tmp ``` ### Re-generating Example Files diff --git a/tools/nosimage/example/example_nosimageprofile_invalid.textproto b/tools/nosimage/example/example_nosimageprofile_invalid.textproto index ed20a9a1650..6cfeb5348ad 100644 --- a/tools/nosimage/example/example_nosimageprofile_invalid.textproto +++ b/tools/nosimage/example/example_nosimageprofile_invalid.textproto @@ -37,29 +37,25 @@ ocpaths: { } ocrpcs: { oc_protocols: { - key: "gnmi.gNMI" + key: "gnmi" value: { - method_name: "Set" - method_name: "Subscribe" + method_name: "gnmi.gNMI.Set" + method_name: "gnmi.gNMI.Subscribe" + method_name: "gnmi.gNMI.Whatsup" version: "0.10.0" } } oc_protocols: { - key: "gnoi.bgp.BGP" + key: "gnoi" value: { - method_name: "ClearBGPNeighbor" - version: "0.1.0" - } - } - oc_protocols: { - key: "gnoi.healthz.Healthz" - value: { - method_name: "Acknowledge" - method_name: "Artifact" - method_name: "Check" - method_name: "Get" - method_name: "List" - version: "1.3.0" + method_name: "gnmi.gNMI.Get" + method_name: "gnoi.bgp.BGP.ClearBGPNeighbor" + method_name: "gnoi.healthz.Healthz.Acknowledge" + method_name: "gnoi.healthz.Healthz.Artifact" + method_name: "gnoi.healthz.Healthz.Check" + method_name: "gnoi.healthz.Healthz.Get" + method_name: "gnoi.healthz.Healthz.List" + version: "0.3.0" } } } diff --git a/tools/nosimage/example/generate_example.go b/tools/nosimage/example/generate_example.go index a4134d5f4f7..00871279ffc 100644 --- a/tools/nosimage/example/generate_example.go +++ b/tools/nosimage/example/generate_example.go @@ -119,27 +119,24 @@ func generateExample(filepath string, valid bool) error { } } return map[string]*rpb.OCProtocol{ - "gnmi.gNMI": { + "gnmi": { Version: "0.10.0", MethodName: []string{ - "Set", - "Subscribe", + "gnmi.gNMI.Set", + "gnmi.gNMI.Subscribe", + "gnmi.gNMI.Whatsup", }, }, - "gnoi.healthz.Healthz": { - Version: "1.3.0", + "gnoi": { + Version: "0.3.0", MethodName: []string{ - "Get", - "List", - "Acknowledge", - "Artifact", - "Check", - }, - }, - "gnoi.bgp.BGP": { - Version: "0.1.0", - MethodName: []string{ - "ClearBGPNeighbor", + "gnoi.healthz.Healthz.Get", + "gnoi.healthz.Healthz.List", + "gnoi.healthz.Healthz.Acknowledge", + "gnoi.healthz.Healthz.Artifact", + "gnoi.healthz.Healthz.Check", + "gnoi.bgp.BGP.ClearBGPNeighbor", + "gnmi.gNMI.Get", }, }, } diff --git a/tools/nosimage/validate/validate.go b/tools/nosimage/validate/validate.go index 19ab7b95187..614fdbe8694 100644 --- a/tools/nosimage/validate/validate.go +++ b/tools/nosimage/validate/validate.go @@ -24,6 +24,7 @@ import ( "path/filepath" "github.com/openconfig/featureprofiles/tools/internal/ocpaths" + "github.com/openconfig/featureprofiles/tools/internal/ocrpcs" "google.golang.org/protobuf/encoding/prototext" npb "github.com/openconfig/featureprofiles/proto/nosimage_go_proto" @@ -44,7 +45,7 @@ func New(fs *flag.FlagSet) *Config { } fs.StringVar(&c.FilePath, "file", "", "txtpb file containing an instance of nosimage.proto data") // TODO(wenovus): Consider allowing using a manual location to avoid git clone. - fs.StringVar(&c.DownloadPath, "download-path", "./", "path into which to download OpenConfig GitHub repos for validation") + fs.StringVar(&c.DownloadPath, "download-path", "./tmp", "path into which to download OpenConfig GitHub repos for validation") return c } @@ -63,7 +64,7 @@ func clonePublicRepo(downloadPath, branch string) (string, error) { } publicPath := filepath.Join(config.DownloadPath, "public") - cmd := exec.Command("git", "clone", "-b", branch, "--single-branch", "--depth", "1", "git@github.com:openconfig/public.git", publicPath) + cmd := exec.Command("git", "clone", "-b", branch, "--single-branch", "--depth", "1", "https://github.com/openconfig/public.git", publicPath) stderr, err := cmd.StderrPipe() if err != nil { return "", err @@ -103,16 +104,34 @@ func main() { os.Exit(1) } + if err := os.MkdirAll(config.DownloadPath, 0750); err != nil { + fmt.Println(fmt.Errorf("cannot create download path directory: %v", config.DownloadPath)) + } publicPath, err := clonePublicRepo(config.DownloadPath, "v"+profile.Ocpaths.GetVersion()) if err != nil { fmt.Println(err) os.Exit(1) } + var hasErr bool paths, invalidPaths, err := ocpaths.ValidatePaths(profile.GetOcpaths().GetOcpaths(), publicPath) if err != nil { fmt.Printf("profile contains %d invalid OCPaths:\n%v", len(invalidPaths), err) + fmt.Println(err) + hasErr = true + } else { + fmt.Printf("profile contains %d valid OCPaths\n", len(paths)) + } + + rpcValidCount, err := ocrpcs.ValidateRPCs(config.DownloadPath, profile.GetOcrpcs().GetOcProtocols()) + if err != nil { + fmt.Println(err) + hasErr = true + } else { + fmt.Printf("profile contains %d valid OCRPCs\n", rpcValidCount) + } + + if hasErr { os.Exit(1) } - fmt.Printf("profile contains %d valid OCPaths\n", len(paths)) }