pkg/repo: Initial cleanup on KEP creation logic

Signed-off-by: Stephen Augustus <[email protected]>

Author: justaugustus

pkg/repo/repo.go

@@ -531,32 +531,3 @@ func (r *Repo) loadKEPFromOldStyle(kepPath string) (*api.Proposal, error) {
 	kep.Name = filepath.Base(kepPath)
 	return kep, nil
 }
-
-func (r *Repo) WriteKEP(kep *api.Proposal) error {
-	b, err := yaml.Marshal(kep)
-	if err != nil {
-		return fmt.Errorf("KEP is invalid: %s", err)
-	}
-
-	if mkErr := os.MkdirAll(
-		filepath.Join(
-			r.ProposalPath,
-			kep.OwningSIG,
-			kep.Name,
-		),
-		os.ModePerm,
-	); mkErr != nil {
-		return errors.Wrapf(mkErr, "creating KEP directory")
-	}
-
-	newPath := filepath.Join(
-		r.ProposalPath,
-		kep.OwningSIG,
-		kep.Name,
-		ProposalMetadataFilename,
-	)
-
-	fmt.Fprintf(r.Out, "writing KEP to %s\n", newPath)
-
-	return ioutil.WriteFile(newPath, b, os.ModePerm)
-}

pkg/repo/write.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package repo
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	"github.com/pkg/errors"
+	"gopkg.in/yaml.v3"
+	"k8s.io/enhancements/api"
+)
+
+func (r *Repo) WriteKEP(kep *api.Proposal) error {
+	b, err := yaml.Marshal(kep)
+	if err != nil {
+		return fmt.Errorf("KEP is invalid: %s", err)
+	}
+
+	sig := kep.OwningSIG
+	kepName := kep.Name
+
+	if sig == "" {
+		return errors.New("owning SIG must be populated")
+	}
+
+	if kepName == "" {
+		return errors.New("KEP name must be populated")
+	}
+
+	if mkErr := os.MkdirAll(
+		filepath.Join(
+			r.ProposalPath,
+			sig,
+			kepName,
+		),
+		os.ModePerm,
+	); mkErr != nil {
+		return errors.Wrapf(mkErr, "creating KEP directory")
+	}
+
+	newPath := filepath.Join(
+		r.ProposalPath,
+		sig,
+		kepName,
+		ProposalMetadataFilename,
+	)
+
+	fmt.Fprintf(r.Out, "writing KEP to %s\n", newPath)
+
+	return ioutil.WriteFile(newPath, b, os.ModePerm)
+}

pkg/proposal/create_test.go renamed to pkg/repo/write_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package proposal
+package repo_test
 
 import (
 	"bytes"
@@ -26,6 +26,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"k8s.io/enhancements/api"
+	"k8s.io/enhancements/pkg/repo"
 	"sigs.k8s.io/yaml"
 )
 
@@ -34,55 +35,83 @@ func TestWriteKep(t *testing.T) {
 		name         string
 		kepFile      string
 		repoPath     string
-		opts         CreateOpts
+		kepName      string
+		sig          string
 		expectedPath string
 		expectError  bool
 	}{
 		{
-			name:     "simple kep",
-			kepFile:  "testdata/valid-kep.yaml",
-			repoPath: "enhancements",
-			opts: CreateOpts{
-				CommonArgs: CommonArgs{
-					Name: "1010-test",
-					SIG:  "sig-auth",
-				},
-			},
+			name:         "simple KEP",
+			kepFile:      "testdata/valid-kep.yaml",
+			repoPath:     "enhancements",
+			kepName:      "1010-test",
+			sig:          "sig-auth",
 			expectedPath: filepath.Join("enhancements", "keps", "sig-auth", "1010-test"),
 			expectError:  false,
 		},
 		{
-			name:     "opts repo path works",
-			kepFile:  "testdata/valid-kep.yaml",
-			repoPath: "",
-			opts: CreateOpts{
-				CommonArgs: CommonArgs{
-					Name:     "1011-test",
-					SIG:      "sig-architecture",
-					RepoPath: "enhancementz",
-				},
-			},
-			expectedPath: filepath.Join("enhancementz", "keps", "sig-architecture", "1011-test"),
-			expectError:  false,
+			name:         "missing KEP name",
+			kepFile:      "testdata/valid-kep.yaml",
+			repoPath:     "enhancements",
+			sig:          "sig-auth",
+			expectedPath: filepath.Join("enhancements", "keps", "sig-auth", "1010-test"),
+			expectError:  true,
+		},
+		{
+			name:         "missing owning SIG",
+			kepFile:      "testdata/valid-kep.yaml",
+			repoPath:     "enhancements",
+			kepName:      "1010-test",
+			expectedPath: filepath.Join("enhancements", "keps", "sig-auth", "1010-test"),
+			expectError:  true,
 		},
 	}
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 			tempDir, err := ioutil.TempDir("", "")
+			mkErr := os.MkdirAll(
+				filepath.Join(
+					tempDir,
+					tc.repoPath,
+					repo.ProposalPathStub,
+					repo.PRRApprovalPathStub,
+				),
+				os.ModePerm,
+			)
+
+			require.Nil(t, mkErr)
+
+			templatePath := filepath.Join(
+				tempDir,
+				tc.repoPath,
+				repo.ProposalPathStub,
+				repo.ProposalTemplatePathStub,
+			)
+
+			mkErr = os.MkdirAll(
+				templatePath,
+				os.ModePerm,
+			)
+
+			require.Nil(t, mkErr)
+
+			templateFile := filepath.Join(templatePath, repo.ProposalFilename)
+			emptyTemplate, fileErr := os.Create(templateFile)
+			require.Nil(t, fileErr)
+			emptyTemplate.Close()
+
 			defer func() {
 				t.Logf("cleanup!")
 				err := os.RemoveAll(tempDir)
 				if err != nil {
 					t.Logf("error cleaning up test: %s", err)
 				}
 			}()
+
 			require.NoError(t, err)
-			repoPath := tc.repoPath
-			if repoPath == "" {
-				repoPath = tc.opts.RepoPath
-			}
 
+			repoPath := tc.repoPath
 			repoPath = filepath.Join(tempDir, repoPath)
 			c := newTestClient(t, repoPath)
 
@@ -93,8 +122,17 @@ func TestWriteKep(t *testing.T) {
 			err = yaml.Unmarshal(b, &p)
 			require.NoError(t, err)
 
-			tc.opts.CommonArgs.RepoPath = repoPath
-			err = c.writeKEP(&p, &tc.opts.CommonArgs)
+			p.OwningSIG = tc.sig
+			p.Name = tc.kepName
+
+			err = c.r.WriteKEP(&p)
+
+			files, readErr := ioutil.ReadDir(c.r.ProposalPath)
+			require.Nil(t, readErr)
+
+			for _, f := range files {
+				t.Logf(f.Name())
+			}
 
 			if tc.expectError {
 				require.Error(t, err)
@@ -106,7 +144,7 @@ func TestWriteKep(t *testing.T) {
 				require.NoError(t, err)
 				require.NotNil(t, dirStat)
 				require.True(t, dirStat.IsDir())
-				p := filepath.Join(computedPath, "kep.yaml")
+				p := filepath.Join(computedPath, repo.ProposalMetadataFilename)
 				fileStat, err := os.Stat(p)
 				require.NoError(t, err)
 				require.NotNil(t, fileStat)
@@ -118,38 +156,48 @@ func TestWriteKep(t *testing.T) {
 type testClient struct {
 	T *testing.T
 	b *bytes.Buffer
-	*Client
+	r *repo.Repo
 }
 
 func newTestClient(t *testing.T, repoPath string) testClient {
 	b := &bytes.Buffer{}
 	tc := testClient{
 		T: t,
 		b: b,
-		Client: &Client{
-			RepoPath: repoPath,
-			Out:      b,
-		},
 	}
 
+	r, err := repo.New(repoPath)
+	require.Nil(t, err)
+
+	r.Out = b
+	tc.r = r
+
 	// TODO: Parameterize
-	tc.addTemplate("kep.yaml")
-	tc.addTemplate("README.md")
+	tc.addTemplate(repo.ProposalMetadataFilename)
+	tc.addTemplate(repo.ProposalFilename)
+
 	return tc
 }
 
 func (tc *testClient) addTemplate(file string) {
-	src := filepath.Join("testdata", "templates", file)
+	src := filepath.Join(
+		"testdata",
+		repo.ProposalPathStub,
+		repo.ProposalTemplatePathStub,
+		file,
+	)
+
 	data, err := ioutil.ReadFile(src)
 	if err != nil {
 		tc.T.Fatal(err)
 	}
 
-	dirPath := filepath.Join(tc.Client.RepoPath, "keps", "NNNN-kep-template")
+	dirPath := filepath.Join(tc.r.BasePath, repo.ProposalPathStub, repo.ProposalTemplatePathStub)
 	err = os.MkdirAll(dirPath, os.ModePerm)
 	if err != nil {
 		tc.T.Fatal(err)
 	}
+
 	dest := filepath.Join(dirPath, file)
 	tc.T.Logf("Writing %s to %s", file, dest)
 	err = ioutil.WriteFile(dest, data, os.ModePerm)