update per code review

Signed-off-by: Patrick Zheng <[email protected]>

internal/file/file.go

@@ -61,8 +61,11 @@ func CopyToDir(src, dst string) (int64, error) {
 }
 
 // FileNameWithoutExtension returns the file name without extension.
+//
 // For example,
+//
 // when input is xyz.exe, output is xyz
+//
 // when input is xyz.tar.gz, output is xyz.tar
 func FileNameWithoutExtension(fileName string) string {
 	return strings.TrimSuffix(fileName, filepath.Ext(fileName))

internal/semver/semver.go

@@ -11,6 +11,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package semver provides functions related to semanic version.
+// This package is based on "golang.org/x/mod/semver"
 package semver
 
 import (

plugin/manager.go

@@ -96,11 +96,12 @@ func (m *CLIManager) List(ctx context.Context) ([]string, error) {
 // On success, the new plugin metadata is returned.
 func (m *CLIManager) Install(ctx context.Context, filePath string, overwrite bool) (*proto.GetMetadataResponse, *proto.GetMetadataResponse, error) {
 	// validate and get new plugin metadata
-	pluginName, err := ExtractPluginNameFromFileName(filepath.Base(filePath))
+	pluginFile := filepath.Base(filePath)
+	pluginName, err := ExtractPluginNameFromFileName(pluginFile)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to get plugin name from file path: %w", err)
 	}
-	if err := validatePluginFileExtensionAgainstOS(filePath, pluginName); err != nil {
+	if err := validatePluginFileExtensionAgainstOS(pluginFile, pluginName); err != nil {
 		return nil, nil, err
 	}
 	newPlugin, err := NewCLIPlugin(ctx, pluginName, filePath)
@@ -146,7 +147,7 @@ func (m *CLIManager) Install(ctx context.Context, filePath string, overwrite boo
 		return nil, nil, fmt.Errorf("failed to copy plugin executable file from %s to %s: %w", filePath, pluginDirPath, err)
 	}
 	// plugin is always executable
-	pluginFilePath := path.Join(pluginDirPath, binName(pluginName))
+	pluginFilePath := path.Join(pluginDirPath, pluginFile)
 	if err := os.Chmod(pluginFilePath, 0700); err != nil {
 		return nil, nil, fmt.Errorf("failed to change the plugin executable file mode: %w", err)
 	}

plugin/plugin.go

@@ -266,15 +266,14 @@ func validate(metadata *proto.GetMetadataResponse) error {
 }
 
 // validatePluginFileExtensionAgainstOS validates if plugin executable file
-// aligns with the runtime OS.
+// extension aligns with the runtime OS.
 //
 // On windows, `.exe` extension is required.
 // On other OS, MUST not have the `.exe` extension.
-func validatePluginFileExtensionAgainstOS(filePath string, pluginName string) error {
-	pluginFile := filepath.Base(filePath)
+func validatePluginFileExtensionAgainstOS(fileName string, pluginName string) error {
 	expectedPluginFile := binName(pluginName)
-	if filepath.Ext(pluginFile) != filepath.Ext(expectedPluginFile) {
-		return fmt.Errorf("invalid plugin file name extension. Expecting file %s, but got %s", expectedPluginFile, pluginFile)
+	if filepath.Ext(fileName) != filepath.Ext(expectedPluginFile) {
+		return fmt.Errorf("invalid plugin file name extension. Expecting file %s, but got %s", expectedPluginFile, fileName)
 	}
 	return nil
 }