Address comments, fix lint failures, and bazel complaints

pkg/kubelet/util/store/BUILD

@@ -7,6 +7,7 @@ go_library(
         "filestore.go",
         "store.go",
     ],
+    importpath = "k8s.io/kubernetes/pkg/kubelet/util/store",
     visibility = ["//visibility:public"],
     deps = ["//pkg/util/filesystem:go_default_library"],
 )
@@ -17,6 +18,7 @@ go_test(
         "filestore_test.go",
         "store_test.go",
     ],
+    importpath = "k8s.io/kubernetes/pkg/kubelet/util/store",
     library = ":go_default_library",
     deps = [
         "//pkg/util/filesystem:go_default_library",

pkg/kubelet/util/store/doc.go

@@ -14,5 +14,5 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// A store interface.
+// Package store hosts a Store interface and its implementations.
 package store // import "k8s.io/kubernetes/pkg/kubelet/util/store"

pkg/kubelet/util/store/filestore.go

@@ -39,6 +39,7 @@ type FileStore struct {
 	filesystem utilfs.Filesystem
 }
 
+// NewFileStore returns an instance of FileStore.
 func NewFileStore(path string, fs utilfs.Filesystem) (Store, error) {
 	if err := ensureDirectory(fs, path); err != nil {
 		return nil, err
@@ -46,6 +47,7 @@ func NewFileStore(path string, fs utilfs.Filesystem) (Store, error) {
 	return &FileStore{directoryPath: path, filesystem: fs}, nil
 }
 
+// Write writes the given data to a file named key.
 func (f *FileStore) Write(key string, data []byte) error {
 	if err := ValidateKey(key); err != nil {
 		return err
@@ -57,17 +59,19 @@ func (f *FileStore) Write(key string, data []byte) error {
 	return writeFile(f.filesystem, f.getPathByKey(key), data)
 }
 
+// Read reads the data from the file named key.
 func (f *FileStore) Read(key string) ([]byte, error) {
 	if err := ValidateKey(key); err != nil {
 		return nil, err
 	}
 	bytes, err := f.filesystem.ReadFile(f.getPathByKey(key))
 	if os.IsNotExist(err) {
-		return bytes, KeyNotFoundError
+		return bytes, ErrKeyNotFound
 	}
 	return bytes, err
 }
 
+// Delete deletes the key file.
 func (f *FileStore) Delete(key string) error {
 	if err := ValidateKey(key); err != nil {
 		return err
@@ -75,6 +79,7 @@ func (f *FileStore) Delete(key string) error {
 	return removePath(f.filesystem, f.getPathByKey(key))
 }
 
+// List returns all keys in the store.
 func (f *FileStore) List() ([]string, error) {
 	keys := make([]string, 0)
 	files, err := f.filesystem.ReadDir(f.directoryPath)
@@ -89,6 +94,7 @@ func (f *FileStore) List() ([]string, error) {
 	return keys, nil
 }
 
+// getPathByKey returns the full path of the file for the key.
 func (f *FileStore) getPathByKey(key string) string {
 	return filepath.Join(f.directoryPath, key)
 }
@@ -110,6 +116,8 @@ func writeFile(fs utilfs.Filesystem, path string, data []byte) (retErr error) {
 		return err
 	}
 
+	tmpPath := tmpFile.Name()
+
 	defer func() {
 		// Close the file.
 		if err := tmpFile.Close(); err != nil {
@@ -119,10 +127,7 @@ func writeFile(fs utilfs.Filesystem, path string, data []byte) (retErr error) {
 				retErr = fmt.Errorf("failed to close temp file after error %v", retErr)
 			}
 		}
-	}()
 
-	tmpPath := tmpFile.Name()
-	defer func() {
 		// Clean up the temp file on error.
 		if retErr != nil && tmpPath != "" {
 			if err := removePath(fs, tmpPath); err != nil {

pkg/kubelet/util/store/filestore_test.go

@@ -31,7 +31,7 @@ func TestFileStore(t *testing.T) {
 	store, err := NewFileStore(path, filesystem.NewFakeFs())
 	assert.NoError(t, err)
 
-	testData := []struct {
+	testCases := []struct {
 		key       string
 		data      string
 		expectErr bool
@@ -69,7 +69,7 @@ func TestFileStore(t *testing.T) {
 	}
 
 	// Test add data.
-	for _, c := range testData {
+	for _, c := range testCases {
 		_, err = store.Read(c.key)
 		assert.Error(t, err)
 
@@ -94,7 +94,7 @@ func TestFileStore(t *testing.T) {
 	assert.Equal(t, keys, []string{"id1", "id2"})
 
 	// Test Delete data
-	for _, c := range testData {
+	for _, c := range testCases {
 		if c.expectErr {
 			continue
 		}
@@ -102,7 +102,7 @@ func TestFileStore(t *testing.T) {
 		err = store.Delete(c.key)
 		assert.NoError(t, err)
 		_, err = store.Read(c.key)
-		assert.EqualValues(t, KeyNotFoundError, err)
+		assert.EqualValues(t, ErrKeyNotFound, err)
 	}
 
 	// Test delete non-existent key.

pkg/kubelet/util/store/store.go

@@ -21,12 +21,21 @@ import (
 	"regexp"
 )
 
-const keyMaxLength = 250
+const (
+	keyMaxLength = 250
 
-var keyRegex = regexp.MustCompile("^[a-zA-Z0-9]+$")
+	keyCharFmt      string = "[A-Za-z0-9]"
+	keyExtCharFmt   string = "[-A-Za-z0-9_.]"
+	qualifiedKeyFmt string = "(" + keyCharFmt + keyExtCharFmt + "*)?" + keyCharFmt
+)
 
 var (
-	KeyNotFoundError = fmt.Errorf("key is not found.")
+	// Key must consist of alphanumeric characters, '-', '_' or '.', and must start
+	// and end with an alphanumeric character.
+	keyRegex = regexp.MustCompile("^" + qualifiedKeyFmt + "$")
+
+	// ErrKeyNotFound is the error returned if key is not found in Store.
+	ErrKeyNotFound = fmt.Errorf("key is not found")
 )
 
 // Store provides the interface for storing keyed data.
@@ -36,7 +45,7 @@ type Store interface {
 	// Write writes data with key.
 	Write(key string, data []byte) error
 	// Read retrieves data with key
-	// Read must return KeyNotFoundError if key is not found.
+	// Read must return ErrKeyNotFound if key is not found.
 	Read(key string) ([]byte, error)
 	// Delete deletes data by key
 	// Delete must not return error if key does not exist
@@ -45,6 +54,8 @@ type Store interface {
 	List() ([]string, error)
 }
 
+// ValidateKey returns an error if the given key does not meet the requirement
+// of the key format and length.
 func ValidateKey(key string) error {
 	if len(key) <= keyMaxLength && keyRegex.MatchString(key) {
 		return nil

pkg/kubelet/util/store/store_test.go

@@ -43,6 +43,14 @@ func TestIsValidKey(t *testing.T) {
 			"a78768279290d33d0b82eaea43cb8346f500057cb5bd250e88c97a5585385d66",
 			true,
 		},
+		{
+			"a7.87-6_8",
+			true,
+		},
+		{
+			"a7.87-677-",
+			false,
+		},
 	}
 
 	for _, tc := range testcases {