Merge branch 'main' into dev/jcalev/ExtensionEvents

Author: jscalev

internal/cmds/cmds.go

@@ -72,7 +72,8 @@ var (
 		"uninstall": CmdUninstall,
 	}
 
-	RunCmd = runCmd
+	RunCmd  = runCmd
+	DataDir = constants.DataDir
 
 	ErrAlreadyProcessed = errors.New("the script configuration has already been processed, will not run again")
 )
@@ -129,12 +130,12 @@ func install(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComm
 		return "", "", err, exitCode
 	}
 
-	if err := os.MkdirAll(constants.DataDir, 0755); err != nil {
+	if err := os.MkdirAll(DataDir, 0755); err != nil {
 		extensionEvents.LogErrorEvent("install", "Failed to create data dir")
 		return "", "", errors.Wrap(err, "failed to create data dir"), constants.ExitCode_CreateDataDirectoryFailed
 	}
 
-	ctx.Log("event", "created data dir", "path", constants.DataDir)
+	ctx.Log("event", "created data dir", "path", DataDir)
 	ctx.Log("event", "installed")
 	extensionEvents.LogInformationalEvent("uninstall", "created data dir")
 	return "", "", nil, constants.ExitCode_Okay
@@ -151,7 +152,6 @@ func uninstall(ctx *log.Context, h types.HandlerEnvironment, report *types.RunCo
 		ctx = ctx.With("path", constants.DataDir)
 		ctx.Log("event", "removing data dir", "path", constants.DataDir)
 		if err := os.RemoveAll(constants.DataDir); err != nil {
-			extensionEvents.LogErrorEvent("uninstall", "failed to delete data directory")
 			return "", "", errors.Wrap(err, "failed to delete data directory"), constants.ExitCode_RemoveDataDirectoryFailed
 		}
 		ctx.Log("event", "removed data dir")

internal/cmds/cmds_test.go

@@ -188,6 +188,7 @@ func Test_checkAndSaveSeqNum(t *testing.T) {
 
 func Test_update_e2e_cmd(t *testing.T) {
 	tempDir, _ := os.MkdirTemp("", "deletecmd")
+	DataDir, _ = os.MkdirTemp("", "datadir")
 	oldVersionDirectory := filepath.Join(tempDir, "Microsoft.CPlat.Core.RunCommandHandlerLinux-1.3.8")
 	newVersionDirectory := filepath.Join(tempDir, "Microsoft.CPlat.Core.RunCommandHandlerLinux-1.3.9")
 	err := os.Mkdir(oldVersionDirectory, 0755)
@@ -239,6 +240,7 @@ func Test_update_e2e_cmd(t *testing.T) {
 
 func Test_udpate_e2e_problematic_version(t *testing.T) {
 	tempDir, _ := os.MkdirTemp("", "deletecmd")
+	DataDir, _ = os.MkdirTemp("", "datadir")
 	oldVersionDirectory := filepath.Join(tempDir, "Microsoft.CPlat.Core.RunCommandHandlerLinux-1.3.17")
 	newVersionDirectory := filepath.Join(tempDir, "Microsoft.CPlat.Core.RunCommandHandlerLinux-1.3.18")
 	err := os.Mkdir(oldVersionDirectory, 0755)
@@ -419,7 +421,7 @@ func Test_runCmd_success(t *testing.T) {
 	require.Nil(t, err)
 	defer os.RemoveAll(dir)
 
-	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, constants.DataDir)
+	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, DataDir)
 	err, exitCode := runCmd(log.NewContext(log.NewNopLogger()), dir, "", &handlersettings.HandlerSettings{
 		PublicSettings: handlersettings.PublicSettings{Source: &handlersettings.ScriptSource{Script: script}},
 	}, metadata)
@@ -445,7 +447,7 @@ func Test_runCmd_fail(t *testing.T) {
 	require.Nil(t, err)
 	defer os.RemoveAll(dir)
 
-	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, constants.DataDir)
+	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, DataDir)
 	err, exitCode := runCmd(log.NewContext(log.NewNopLogger()), dir, "", &handlersettings.HandlerSettings{
 		PublicSettings: handlersettings.PublicSettings{Source: &handlersettings.ScriptSource{Script: "non-existing-cmd"}},
 	}, metadata)
@@ -676,7 +678,7 @@ func Test_TreatFailureAsDeploymentFailureIsTrue_Fails(t *testing.T) {
 	require.Nil(t, err)
 	defer os.RemoveAll(dir)
 
-	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, constants.DataDir)
+	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, DataDir)
 	err, exitCode := runCmd(log.NewContext(log.NewNopLogger()), dir, "", &handlersettings.HandlerSettings{
 		PublicSettings: handlersettings.PublicSettings{Source: &handlersettings.ScriptSource{Script: script}, TreatFailureAsDeploymentFailure: true},
 	}, metadata)
@@ -695,7 +697,7 @@ func Test_TreatFailureAsDeploymentFailureIsTrue_SimpleScriptSucceeds(t *testing.
 	require.Nil(t, err)
 	defer os.RemoveAll(dir)
 
-	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, constants.DataDir)
+	metadata := types.NewRCMetadata("extName", 0, constants.DownloadFolder, DataDir)
 	err, exitCode := runCmd(log.NewContext(log.NewNopLogger()), dir, "", &handlersettings.HandlerSettings{
 		PublicSettings: handlersettings.PublicSettings{Source: &handlersettings.ScriptSource{Script: script}, TreatFailureAsDeploymentFailure: false},
 	}, metadata)

pkg/download/blob_test.go

@@ -78,6 +78,18 @@ func Test_blobDownload_fails_badCreds(t *testing.T) {
 		Container:   "foocontainer",
 	})
 
+	mockResponse := &http.Response{
+		StatusCode: http.StatusForbidden,
+		Status:     "The chipmunks do not like you and thus returned a 403",
+	}
+
+	original := MakeHttpRequest
+	defer func() { MakeHttpRequest = original }()
+
+	MakeHttpRequest = func(*http.Request) (*http.Response, error) {
+		return mockResponse, nil
+	}
+
 	status, _, err := Download(testctx, d)
 	require.NotNil(t, err)
 	require.Contains(t, err.Error(), "Please verify the machine has network connectivity")
@@ -94,6 +106,18 @@ func Test_blobDownload_fails_badRequest(t *testing.T) {
 			Container:   "foocontainer",
 		}}}
 
+	mockResponse := &http.Response{
+		StatusCode: http.StatusBadRequest,
+		Status:     "The chipmunks don't understand you. 400.",
+	}
+
+	original := MakeHttpRequest
+	defer func() { MakeHttpRequest = original }()
+
+	MakeHttpRequest = func(*http.Request) (*http.Response, error) {
+		return mockResponse, nil
+	}
+
 	status, _, err := Download(testctx, d)
 	require.NotNil(t, err)
 	require.Contains(t, err.Error(), "parts of the request were incorrectly formatted, missing, and/or invalid")

pkg/download/downloader.go

@@ -27,6 +27,8 @@ const (
 )
 
 var (
+	MakeHttpRequest = HttpClientDo
+
 	// httpClient is the default client to be used in downloading files from
 	// Internet. http.Get() uses a client without timeouts (http.DefaultClient)
 	// so it is dangerous to use it for downloading files from the Internet.
@@ -43,6 +45,10 @@ var (
 		}}
 )
 
+func HttpClientDo(request *http.Request) (*http.Response, error) {
+	return httpClient.Do(request)
+}
+
 // Download retrieves a response body and checks the response status code to see
 // if it is 200 OK and then returns the response body. It issues a new request
 // every time called. It is caller's responsibility to close the response body.
@@ -56,7 +62,7 @@ func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, erro
 		ctx.Log("info", fmt.Sprintf("starting download with client request ID %s", requestID))
 	}
 
-	response, err := httpClient.Do(request)
+	response, err := MakeHttpRequest(request)
 	if err != nil {
 		err = urlutil.RemoveUrlFromErr(err)
 		return -1, nil, errors.Wrapf(err, "http request failed")