Merge pull request kubernetes#2010 from johnbelamaric/kepctl-nested-dirs

kepctl: fix query when KEPs are nested in directories

Author: k8s-ci-robot

go.sum

@@ -1060,6 +1060,7 @@ github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/syndtr/gocapability v0.0.0-20170704070218-db04d3cc01c8/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=

pkg/kepctl/kepctl.go

@@ -162,13 +162,19 @@ func validateKEP(p *keps.Proposal) error {
 	return nil
 }
 
-func findLocalKEPs(repoPath string, sig string) ([]string, error) {
+func findLocalKEPMeta(repoPath, sig string) ([]string, error) {
 	rootPath := filepath.Join(
 		repoPath,
 		"keps",
 		sig)
 
 	keps := []string{}
+
+	// if the sig doesn't have a dir, it has no KEPs
+	if _, err := os.Stat(rootPath); os.IsNotExist(err) {
+		return keps, nil
+	}
+
 	err := filepath.Walk(rootPath, func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -177,7 +183,7 @@ func findLocalKEPs(repoPath string, sig string) ([]string, error) {
 			return nil
 		}
 		if info.Name() == "kep.yaml" {
-			keps = append(keps, filepath.Base(filepath.Dir(path)))
+			keps = append(keps, path)
 			return filepath.SkipDir
 		}
 		if filepath.Ext(path) != ".md" {
@@ -186,14 +192,42 @@ func findLocalKEPs(repoPath string, sig string) ([]string, error) {
 		if info.Name() == "README.md" {
 			return nil
 		}
-		keps = append(keps, info.Name()[0:len(info.Name())-3])
+		keps = append(keps, path)
 		return nil
 	})
 
 	return keps, err
 }
 
-func (c *Client) findKEPPullRequests(sig string) ([]*keps.Proposal, error) {
+func (c *Client) loadLocalKEPs(repoPath, sig string) ([]*keps.Proposal) {
+	// KEPs in the local filesystem
+	files, err := findLocalKEPMeta(repoPath, sig)
+	if err != nil {
+		fmt.Fprintf(c.Err, "error searching for local KEPs from %s: %s\n", sig, err)
+	}
+
+	var allKEPs []*keps.Proposal
+	for _, k := range files {
+		if filepath.Ext(k) == ".yaml" {
+			kep, err := c.loadKEPFromYaml(k)
+			if err != nil {
+				fmt.Fprintf(c.Err, "error reading KEP %s: %s\n", k, err)
+			} else {
+				allKEPs = append(allKEPs, kep)
+			}
+		} else {
+			kep, err := c.loadKEPFromOldStyle(k)
+			if err != nil {
+				fmt.Fprintf(c.Err, "error reading KEP %s: %s\n", k, err)
+			} else {
+				allKEPs = append(allKEPs, kep)
+			}
+		}
+	}
+	return allKEPs
+}
+
+func (c *Client) loadKEPPullRequests(sig string) ([]*keps.Proposal, error) {
 	var auth *http.Client
 	ctx := context.Background()
 	if c.Token != "" {
@@ -301,19 +335,10 @@ func (c *Client) readKEP(repoPath string, sig, name string) (*keps.Proposal, err
 		sig,
 		name,
 		"kep.yaml")
+
 	_, err := os.Stat(kepPath)
 	if err == nil {
-		b, err := ioutil.ReadFile(kepPath)
-		if err != nil {
-			return nil, fmt.Errorf("unable to read KEP metadata: %s", err)
-		}
-		var p keps.Proposal
-		err = yaml.Unmarshal(b, &p)
-		if err != nil {
-			return nil, fmt.Errorf("unable to load KEP metadata: %s", err)
-		}
-		p.Name = name
-		return &p, nil
+		return c.loadKEPFromYaml(kepPath)
 	}
 
 	// No kep.yaml, treat as old-style KEP
@@ -322,6 +347,25 @@ func (c *Client) readKEP(repoPath string, sig, name string) (*keps.Proposal, err
 		"keps",
 		sig,
 		name) + ".md"
+
+	return c.loadKEPFromOldStyle(kepPath)
+}
+
+func (c *Client) loadKEPFromYaml(kepPath string) (*keps.Proposal, error) {
+	b, err := ioutil.ReadFile(kepPath)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read KEP metadata: %s", err)
+	}
+	var p keps.Proposal
+	err = yaml.Unmarshal(b, &p)
+	if err != nil {
+		return nil, fmt.Errorf("unable to load KEP metadata: %s", err)
+	}
+	p.Name = filepath.Base(filepath.Dir(kepPath))
+	return &p, nil
+}
+
+func (c *Client) loadKEPFromOldStyle(kepPath string) (*keps.Proposal, error) {
 	b, err := ioutil.ReadFile(kepPath)
 	if err != nil {
 		return nil, fmt.Errorf("no kep.yaml, but failed to read as old-style KEP: %s", err)
@@ -333,7 +377,7 @@ func (c *Client) readKEP(repoPath string, sig, name string) (*keps.Proposal, err
 	if kep.Error != nil {
 		return nil, fmt.Errorf("kep is invalid: %s", kep.Error)
 	}
-	kep.Name = name + ".md"
+	kep.Name = filepath.Base(kepPath)
 	return kep, nil
 }
 

pkg/kepctl/kepctl_test.go

@@ -41,7 +41,7 @@ func TestValidate(t *testing.T) {
 		{
 			name: "invalid kep fails valdiate for owning-sig",
 			file: "testdata/invalid-kep.yaml",
-			err:  fmt.Errorf(`kep is invalid: error validating KEP metadata: "owning-sig" must be one of (committee-code-of-conduct,committee-product-security,committee-steering,sig-api-machinery,sig-apps,sig-architecture,sig-auth,sig-autoscaling,sig-cli,sig-cloud-provider,sig-cluster-lifecycle,sig-contributor-experience,sig-docs,sig-instrumentation,sig-multicluster,sig-network,sig-node,sig-release,sig-scalability,sig-scheduling,sig-service-catalog,sig-storage,sig-testing,sig-ui,sig-usability,sig-windows,ug-big-data,ug-vmware-users,wg-api-expression,wg-component-standard,wg-data-protection,wg-iot-edge,wg-k8s-infra,wg-lts,wg-machine-learning,wg-multitenancy,wg-naming,wg-policy,wg-security-audit) but it is a string: sig-awesome`),
+			err:  fmt.Errorf(`kep is invalid: error validating KEP metadata: "owning-sig" must be one of (committee-code-of-conduct,committee-product-security,committee-steering,sig-api-machinery,sig-apps,sig-architecture,sig-auth,sig-autoscaling,sig-cli,sig-cloud-provider,sig-cluster-lifecycle,sig-contributor-experience,sig-docs,sig-instrumentation,sig-multicluster,sig-network,sig-node,sig-release,sig-scalability,sig-scheduling,sig-security,sig-service-catalog,sig-storage,sig-testing,sig-ui,sig-usability,sig-windows,ug-big-data,ug-vmware-users,wg-api-expression,wg-component-standard,wg-data-protection,wg-iot-edge,wg-k8s-infra,wg-lts,wg-multitenancy,wg-naming,wg-policy,wg-security-audit) but it is a string: sig-awesome`),
 		},
 	}
 
@@ -70,29 +70,25 @@ func TestFindLocalKEPs(t *testing.T) {
 	}{
 		{
 			"sig-architecture",
-			[]string{"123-newstyle", "20200115-kubectl-diff"},
+			[]string{"123-newstyle", "20200115-kubectl-diff.md"},
 		},
 		{
 			"sig-sig",
 			[]string{},
 		},
 	}
 
+	c, _ := New("testdata")
 	for i, tc := range testcases {
-		k, err := findLocalKEPs("testdata", tc.sig)
-		if err != nil {
-			t.Errorf("Test case %d: expected no error but got %s", i, err)
-			continue
-		}
+		k := c.loadLocalKEPs("testdata", tc.sig)
 		if len(k) != len(tc.keps) {
-			t.Errorf("Test case %d: expected %s but got %s", i, tc.keps, k)
+			t.Errorf("Test case %d: expected %d but got %d", i, len(tc.keps), len(k))
 			continue
 		}
 		for j, kn := range k {
-			if kn != tc.keps[j] {
-				t.Errorf("Test case %d: expected %s but got %s", i, tc.keps[j], kn)
+			if kn.Name != tc.keps[j] {
+				t.Errorf("Test case %d: expected %s but got %s", i, tc.keps[j], kn.Name)
 			}
 		}
 	}
-	findLocalKEPs("testdata", "sig-architecture")
 }

pkg/kepctl/query.go

@@ -66,23 +66,11 @@ func (c *Client) Query(opts QueryOpts) error {
 	// load the KEPs for each listed SIG
 	for _, sig := range opts.SIG {
 		// KEPs in the local filesystem
-		names, err := findLocalKEPs(repoPath, sig)
-		if err != nil {
-			fmt.Fprintf(c.Err, "error searching for local KEPs from %s: %s\n", sig, err)
-		}
-
-		for _, k := range names {
-			kep, err := c.readKEP(repoPath, sig, k)
-			if err != nil {
-				fmt.Fprintf(c.Err, "error reading KEP %s: %s\n", k, err)
-			} else {
-				allKEPs = append(allKEPs, kep)
-			}
-		}
+		allKEPs = append(allKEPs, c.loadLocalKEPs(repoPath, sig)...)
 
 		// Open PRs; existing KEPs with open PRs will be shown twice
 		if opts.IncludePRs {
-			prKeps, err := c.findKEPPullRequests(sig)
+			prKeps, err := c.loadKEPPullRequests(sig)
 			if err != nil {
 				fmt.Fprintf(c.Err, "error searching for KEP PRs from %s: %s\n", sig, err)
 			}