diff --git a/go.mod b/go.mod index f76efeb..baffaa1 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ toolchain go1.24.5 require ( github.com/Azure/azure-extension-foundation v0.0.0-20250620154556-caff9e3c3c5c - github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f + github.com/Azure/azure-extension-platform v0.0.0-20260107210613-2a62cc200c34 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.0 diff --git a/go.sum b/go.sum index 6ee1b20..2f221f5 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,8 @@ github.com/Azure/azure-extension-platform v0.0.0-20240610175536-404c704f82f8 h1: github.com/Azure/azure-extension-platform v0.0.0-20240610175536-404c704f82f8/go.mod h1:nEQQIC3RKmMnpdc+RakYHIdu556jdcHv67ML8PdsQeQ= github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f h1:ddsUz/suc9txCMz/xWOslqNMvzhbWFMTflUrbcMNoSw= github.com/Azure/azure-extension-platform v0.0.0-20250107200156-aa20f765d49f/go.mod h1:0458BvQsi5ch6kn+KZtI5m88Z3L9UFXdoY1+6nKdivY= +github.com/Azure/azure-extension-platform v0.0.0-20260107210613-2a62cc200c34 h1:7bEC4DJC4w0gx7SBy7M7Q2qi6ckmHcnnlFJzo+X/gi4= +github.com/Azure/azure-extension-platform v0.0.0-20260107210613-2a62cc200c34/go.mod h1:0458BvQsi5ch6kn+KZtI5m88Z3L9UFXdoY1+6nKdivY= github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0hS+6+I79yEDJBqVNcqUzU= github.com/Azure/azure-sdk-for-go v68.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.16.0 h1:JZg6HRh6W6U4OLl6lk7BZ7BLisIzM9dG1R50zUk9C/M= diff --git a/internal/cmds/cmds.go b/internal/cmds/cmds.go index 864c001..87620ba 100755 --- a/internal/cmds/cmds.go +++ b/internal/cmds/cmds.go @@ -19,6 +19,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/azure-sdk-for-go/sdk/azcore/streaming" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/appendblob" @@ -133,7 +134,7 @@ func install(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComm if err := os.MkdirAll(DataDir, 0755); err != nil { errMessage := fmt.Sprintf("Failed to create data dir: %v due to: %v", DataDir, err) extensionEvents.LogErrorEvent("install", errMessage) - return "", "", errors.Wrap(err, errMessage), constants.ExitCode_CreateDataDirectoryFailed + return "", "", errors.Wrap(err, errMessage), constants.FileSystem_CreateDataDirectoryFailed } ctx.Log("event", "created data dir", "path", DataDir) @@ -155,7 +156,7 @@ func uninstall(ctx *log.Context, h types.HandlerEnvironment, report *types.RunCo if err := os.RemoveAll(DataDir); err != nil { errMessage := fmt.Sprintf("Failed to delete data directory: %v due to: %v", DataDir, err) extensionEvents.LogErrorEvent("uninstall", errMessage) - return "", "", errors.Wrap(err, errMessage), constants.ExitCode_RemoveDataDirectoryFailed + return "", "", errors.Wrap(err, errMessage), constants.FileSystem_RemoveDataDirectoryFailed } ctx.Log("event", "removed data dir") extensionEvents.LogInformationalEvent("uninstall", fmt.Sprintf("removed data dir %v", DataDir)) @@ -190,7 +191,7 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if err1 != nil { errMessage := fmt.Sprintf("Failed to get configuration: %v", err1) extensionEvents.LogErrorEvent("enable", errMessage) - return "", "", errors.Wrap(err1, "failed to get configuration"), constants.ExitCode_GetHandlerSettingsFailed + return "", "", err1, constants.CommandExecution_BadConfig } exitCode, err := immediatecmds.Enable(ctx, h, metadata.ExtName, metadata.SeqNum, cfg, extensionEvents) @@ -201,23 +202,23 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma } dir := filepath.Join(metadata.DownloadPath, fmt.Sprintf("%d", metadata.SeqNum)) - scriptFilePath, err := downloadScript(ctx, dir, &cfg) - if err != nil { + scriptFilePath, ewc := downloadScript(ctx, dir, &cfg) + if ewc != nil { errMessage := fmt.Sprintf("Failed to download script: %v due to: %v", download.GetUriForLogging(cfg.ScriptURI()), err) extensionEvents.LogErrorEvent("enable", errMessage) return "", "", - errors.Wrap(err, fmt.Sprintf("File downloads failed. Use either a public script URI that points to .sh file, Azure storage blob SAS URI or storage blob accessible by a managed identity and retry. If managed identity is used, make sure it has been given access to container of storage blob '%s' with 'Storage Blob Data Reader' role assignment. In case of user-assigned identity, make sure you add it under VM's identity. For more info, refer https://aka.ms/RunCommandManagedLinux", download.GetUriForLogging(cfg.ScriptURI()))), - constants.ExitCode_ScriptBlobDownloadFailed + vmextension.CreateWrappedErrorWithClarification(ewc, fmt.Sprintf("File downloads failed. Use either a public script URI that points to .sh file, Azure storage blob SAS URI or storage blob accessible by a managed identity and retry. If managed identity is used, make sure it has been given access to container of storage blob '%s' with 'Storage Blob Data Reader' role assignment. In case of user-assigned identity, make sure you add it under VM's identity. For more info, refer https://aka.ms/RunCommandManagedLinux", download.GetUriForLogging(cfg.ScriptURI()))), + constants.FileDownload_GenericError } - err = downloadArtifacts(ctx, dir, &cfg) - if err != nil { - errMessage := fmt.Sprintf("Failed to download artifacts: %v", err) + ewc = downloadArtifacts(ctx, dir, &cfg) + if ewc != nil { + errMessage := fmt.Sprintf("Failed to download artifacts: %v", ewc) extensionEvents.LogErrorEvent("enable", errMessage) return "", "", - errors.Wrap(err, "Artifact downloads failed. Use either a public artifact URI that points to .sh file, Azure storage blob SAS URI, or storage blob accessible by a managed identity and retry."), - constants.ExitCode_DownloadArtifactFailed + vmextension.CreateWrappedErrorWithClarification(ewc, "Artifact downloads failed. Use either a public artifact URI that points to .sh file, Azure storage blob SAS URI, or storage blob accessible by a managed identity and retry."), + constants.ArtifactDownload_GenericError } blobCreateOrReplaceError := "Error creating AppendBlob '%s' using SAS token or Managed identity. Please use a valid blob SAS URI with [read, append, create, write] permissions OR managed identity. If managed identity is used, make sure Azure blob and identity exist, and identity has been given access to storage blob's container with 'Storage Blob Data Contributor' role assignment. In case of user-assigned identity, make sure you add it under VM's identity and provide outputBlobUri / errorBlobUri and corresponding clientId in outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). In case of system-assigned identity, do not use outputBlobManagedIdentity / errorBlobManagedIdentity parameter(s). For more info, refer https://aka.ms/RunCommandManagedLinux" @@ -235,8 +236,8 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if outputBlobAppendCreateOrReplaceError != nil { return "", "", - errors.Wrap(outputBlobAppendCreateOrReplaceError, fmt.Sprintf(blobCreateOrReplaceError, cfg.OutputBlobURI)), - constants.ExitCode_BlobCreateOrReplaceFailed + vmextension.CreateWrappedErrorWithClarification(outputBlobAppendCreateOrReplaceError, fmt.Sprintf(blobCreateOrReplaceError, cfg.OutputBlobURI)), + constants.AppendBlobCreation_Other } } @@ -253,8 +254,8 @@ func enable(ctx *log.Context, h types.HandlerEnvironment, report *types.RunComma if errorBlobAppendCreateOrReplaceError != nil { return "", "", - errors.Wrap(errorBlobAppendCreateOrReplaceError, fmt.Sprintf(blobCreateOrReplaceError, cfg.ErrorBlobURI)), - constants.ExitCode_BlobCreateOrReplaceFailed + vmextension.CreateWrappedErrorWithClarification(errorBlobAppendCreateOrReplaceError, fmt.Sprintf(blobCreateOrReplaceError, cfg.ErrorBlobURI)), + constants.AppendBlobCreation_Other } } @@ -563,7 +564,7 @@ func doRehydrateMrSeqFilesForProblematicUpgrades(ctx *log.Context, oldExtensionD } // Copy files like *.mrseq (Most Recently executed Sequence number), .status files from old extension version to new extension version during update. -func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory string, extensionEvents *extensionevents.ExtensionEventManager) (*list.List, error) { +func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory string, extensionEvents *extensionevents.ExtensionEventManager) (*list.List, *vmextension.ErrorWithClarification) { newExtensionVersion := os.Getenv(constants.ExtensionVersionEnvName) oldExtensionVersion := os.Getenv(constants.ExtensionVersionUpdatingFromEnvName) @@ -587,7 +588,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory if errr != nil { errMessage := fmt.Sprintf("Failed to create directory '%s'", newExtensionDirectory) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return nil, errors.Wrap(errr, errMessage) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_CouldNotCreateStatusDirectory, fmt.Errorf("Failed to create directory '%s': %v", newExtensionDirectory, err)) } } } @@ -595,7 +596,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory if oldExtensionDirectory == "" || newExtensionDirectory == "" { errMessage := "oldExtesionDirectory or newExtensionDirectory is empty" extensionEvents.LogErrorEvent("copyfiles", errMessage) - return nil, errors.New(errMessage) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_ExtensionDirectoryNameEmpty, errors.New(errMessage)) } // Check if the directory exists @@ -604,7 +605,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory errMessage := fmt.Sprintf("could not open sourceDirectory %s", oldExtensionDirectory) ctx.Log("message", errMessage) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return nil, errors.Wrap(err, errMessage) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_CouldNotOpenSubdirectory, fmt.Errorf("%s: %v", errMessage, err)) } directoryEntries, err := sourceDirectoryFDRef.ReadDir(0) @@ -612,7 +613,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory errMessage := fmt.Sprintf("could not read directory entries from sourceDirectory %s", oldExtensionDirectory) ctx.Log("message", errMessage) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return nil, errors.Wrap(err, errMessage) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_CouldNotReadDirectoryEntries, fmt.Errorf("%s: %v", errMessage, err)) } numberOfFilesMigrated := 0 @@ -630,7 +631,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory errMessage := "Failed to open '%s' file '%s' for reading. Contact ICM team AzureRT\\Extensions for this service error." ctx.Log("message", fmt.Sprintf(errMessage, fileExtensionSuffix, sourceFileFullPath)) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return fileNamesMigrated, errors.Wrapf(sourceFileOpenError, errMessage) + return fileNamesMigrated, vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToOpenFileForReading, fmt.Errorf("%s: %v", errMessage, sourceFileOpenError)) } defer sourceFile.Close() @@ -639,7 +640,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory errMessage := "Failed to create '%s' file '%s'. Contact ICM team AzureRT\\Extensions for this service error." ctx.Log("message", fmt.Sprintf(errMessage, fileExtensionSuffix, destinationFileFullPath)) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return fileNamesMigrated, errors.Wrapf(destFileCreateError, errMessage) + return fileNamesMigrated, vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToCreateFile, fmt.Errorf("%s: %v", errMessage, destFileCreateError)) } defer destFile.Close() @@ -649,7 +650,7 @@ func copyFiles(ctx log.Logger, fileExtensionSuffix string, extensionSubdirectory fileExtensionSuffix, sourceFileFullPath, destinationFileFullPath) ctx.Log("message", errMessage) extensionEvents.LogErrorEvent("copyfiles", errMessage) - return fileNamesMigrated, errors.Wrapf(copyError, errMessage) + return fileNamesMigrated, vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToCopyFile, fmt.Errorf("%s: %v", errMessage, copyError)) } else { message := fmt.Sprintf("File '%s' was copied successfully to '%s'", sourceFileFullPath, destinationFileFullPath) ctx.Log("message", message) @@ -782,12 +783,12 @@ func createDummyStatusFilesIfNeeded(ctx log.Logger, mrseqFilesNameList *list.Lis // downloadScript downloads the script file specified in cfg into dir (creates if does // not exist) and takes storage credentials specified in cfg into account. -func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings) (string, error) { +func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings) (string, *vmextension.ErrorWithClarification) { // - prepare the output directory for files and the command output // - create the directory if missing ctx.Log("event", "creating output directory", "path", dir) if err := os.MkdirAll(dir, 0700); err != nil { - return "", errors.Wrap(err, "failed to prepare output directory") + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CreateDirectoryFailure, err) } ctx.Log("event", "created output directory") @@ -803,7 +804,7 @@ func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSe file, err := files.DownloadAndProcessScript(ctx, scriptURI, dir, cfg) if err != nil { ctx.Log("event", "download failed", "error", err) - return "", errors.Wrapf(err, "failed to download file %s. ", scriptURI) + return "", err } scriptFilePath = file ctx.Log("event", "download complete", "output", dir) @@ -811,7 +812,7 @@ func downloadScript(ctx *log.Context, dir string, cfg *handlersettings.HandlerSe return scriptFilePath, nil } -func downloadArtifacts(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings) error { +func downloadArtifacts(ctx *log.Context, dir string, cfg *handlersettings.HandlerSettings) *vmextension.ErrorWithClarification { artifacts, err := cfg.ReadArtifacts() if err != nil { return err @@ -827,7 +828,7 @@ func downloadArtifacts(ctx *log.Context, dir string, cfg *handlersettings.Handle filePath, err := files.DownloadAndProcessArtifact(ctx, dir, &artifacts[i]) if err != nil { ctx.Log("events", "Failed to download artifact", err, "artifact", artifacts[i].ArtifactUri) - return errors.Wrapf(err, "failed to download artifact %s", artifacts[i].ArtifactUri) + return vmextension.CreateWrappedErrorWithClarification(err, "Failed to download artifact") } ctx.Log("event", "Downloaded artifact complete", "file", filePath) @@ -837,7 +838,7 @@ func downloadArtifacts(ctx *log.Context, dir string, cfg *handlersettings.Handle } // runCmd runs the command (extracted from cfg) in the given dir (assumed to exist). -func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (err error, exitCode int) { +func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (err *vmextension.ErrorWithClarification, exitCode int) { ctx.Log("event", "executing command", "output", dir) var scenario string @@ -849,7 +850,7 @@ func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlerset err := files.SaveScriptFile(scriptFilePath, cfg.Script()) if err != nil { ctx.Log("event", "failed to save script to file", "error", err, "file", scriptFilePath) - return errors.Wrap(err, "failed to save script to file"), constants.ExitCode_SaveScriptFailed + return err, constants.FileDownload_UnableToWriteFile } } else if cfg.ScriptURI() != "" { // If scriptUri is specified then cmd should start it @@ -875,18 +876,18 @@ func runCmd(ctx *log.Context, dir string, scriptFilePath string, cfg *handlerset if err != nil { ctx.Log("event", "failed to execute command", "error", err, "output", dir) - return errors.Wrap(err, "failed to execute command"), exitCode + return err, exitCode } ctx.Log("event", "executed command", "output", dir) return nil, constants.ExitCode_Okay } // base64 decode and optionally GZip decompress a script -func decodeScript(script string) (string, string, error) { +func decodeScript(script string) (string, string, *vmextension.ErrorWithClarification) { // scripts must be base64 encoded s, err := base64.StdEncoding.DecodeString(script) if err != nil { - return "", "", errors.Wrap(err, "failed to decode script") + return "", "", vmextension.NewErrorWithClarificationPtr(constants.Script_FailedToDecode, errors.Wrap(err, "failed to decode script")) } // scripts may be gzip'ed @@ -900,14 +901,14 @@ func decodeScript(script string) (string, string, error) { n, err := io.Copy(w, r) if err != nil { - return "", "", errors.Wrap(err, "failed to decompress script") + return "", "", vmextension.NewErrorWithClarificationPtr(constants.Script_FailedToDecompress, errors.Wrap(err, "failed to decompress script")) } w.Flush() return buf.String(), fmt.Sprintf("%d;%d;gzip=1", len(script), n), nil } -func createOrReplaceAppendBlobUsingManagedIdentity(blobUri string, managedIdentity *handlersettings.RunCommandManagedIdentity) (*appendblob.Client, error) { +func createOrReplaceAppendBlobUsingManagedIdentity(blobUri string, managedIdentity *handlersettings.RunCommandManagedIdentity) (*appendblob.Client, *vmextension.ErrorWithClarification) { var ID string = "" var miCred *azidentity.ManagedIdentityCredential = nil var miCredError error = nil @@ -916,7 +917,7 @@ func createOrReplaceAppendBlobUsingManagedIdentity(blobUri string, managedIdenti if managedIdentity.ClientId != "" { ID = managedIdentity.ClientId } else if managedIdentity.ObjectId != "" { //ObjectId is not supported by azidentity.NewManagedIdentityCredential - return nil, errors.New("Managed identity's ObjectId is not supported. Use ClientId instead") + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_ObjectIdNotSupported, errors.New("Managed identity's ObjectId is not supported. Use ClientId instead")) } } @@ -932,16 +933,16 @@ func createOrReplaceAppendBlobUsingManagedIdentity(blobUri string, managedIdenti if miCredError == nil { appendBlobClient, appendBlobNewClientError = appendblob.NewClient(blobUri, miCred, nil) if appendBlobNewClientError != nil { - return nil, errors.Wrap(appendBlobNewClientError, fmt.Sprintf("Error Creating client to Append Blob '%s'. Make sure you are using Append blob. Other types of blob such as PageBlob, BlockBlob are not supported types.", download.GetUriForLogging(blobUri))) + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_ClientError, errors.Wrap(appendBlobNewClientError, fmt.Sprintf("Error Creating client to Append Blob '%s'. Make sure you are using Append blob. Other types of blob such as PageBlob, BlockBlob are not supported types.", download.GetUriForLogging(blobUri)))) } else { // Create or Replace Append blob. If AppendBlob already exists, blob gets cleared. _, createAppendBlobError := appendBlobClient.Create(context.Background(), nil) if createAppendBlobError != nil { - return nil, errors.Wrap(createAppendBlobError, fmt.Sprintf("Error creating or replacing the Append blob '%s'. Make sure you are using Append blob. Other types of blob such as PageBlob, BlockBlob are not supported types.", download.GetUriForLogging(blobUri))) + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_Other, errors.Wrap(createAppendBlobError, fmt.Sprintf("Error creating or replacing the Append blob '%s'. Make sure you are using Append blob. Other types of blob such as PageBlob, BlockBlob are not supported types.", download.GetUriForLogging(blobUri)))) } } } else { - return nil, errors.Wrap(miCredError, "Error while retrieving managed identity credential") + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_InvalidMsi, errors.Wrap(miCredError, "Error while retrieving managed identity credential")) } return appendBlobClient, nil @@ -965,7 +966,6 @@ func createOrReplaceAppendBlob(blobUri string, sasToken string, managedIdentity // Try to create or replace output blob using managed identity. if sasToken == "" || blobSASTokenError != nil { - blobAppendClient, blobAppendClientError = createOrReplaceAppendBlobUsingManagedIdentity(blobUri, managedIdentity) } @@ -978,7 +978,7 @@ func createOrReplaceAppendBlob(blobUri string, sasToken string, managedIdentity } else { er = blobAppendClientError } - return nil, nil, errors.Wrap(er, "Creating or Replacing append blob failed.") + return nil, nil, er } } return blobSASRef, blobAppendClient, nil diff --git a/internal/cmds/cmds_test.go b/internal/cmds/cmds_test.go index 205c5ef..002c2e1 100755 --- a/internal/cmds/cmds_test.go +++ b/internal/cmds/cmds_test.go @@ -12,10 +12,14 @@ import ( "strings" "testing" + osexec "os/exec" + "github.com/Azure/azure-extension-platform/pkg/extensionevents" "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/exec" "github.com/Azure/run-command-handler-linux/internal/files" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/internal/settings" @@ -377,7 +381,7 @@ func enable_extension(t *testing.T, fakeEnv types.HandlerEnvironment, tempDir st err = encoder.Encode(handlerSettings) require.Nil(t, err, "Could not serialze settings file") - RunCmd = func(ctx *log.Context, dir, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (error, int) { + RunCmd = func(ctx *log.Context, dir, scriptFilePath string, cfg *handlersettings.HandlerSettings, metadata types.RCMetadata) (*vmextension.ErrorWithClarification, int) { wasCalled = true return nil, 0 // mock behavior } @@ -435,6 +439,13 @@ func Test_runCmd_success(t *testing.T) { require.Nil(t, err) defer os.RemoveAll(dir) + orig := exec.FnRunCommand + defer func() { exec.FnRunCommand = orig }() + exec.FnRunCommand = func(_ *osexec.Cmd) error { + // Success + return nil + } + 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}}, @@ -582,7 +593,7 @@ func Test_downloadArtifactsFail(t *testing.T) { }) require.NotNil(t, err) - require.Contains(t, err.Error(), "failed to download artifact") + require.Contains(t, err.Error(), "Failed to download artifact") } func Test_downloadArtifacts(t *testing.T) { @@ -637,7 +648,7 @@ func Test_decodeScript(t *testing.T) { testSubject := "bHMK" s, info, err := decodeScript(testSubject) - require.NoError(t, err) + require.Nil(t, err) require.Equal(t, info, "4;3;gzip=0") require.Equal(t, s, "ls\n") } @@ -646,7 +657,7 @@ func Test_decodeScriptGzip(t *testing.T) { testSubject := "H4sIACD731kAA8sp5gIAfShLWgMAAAA=" s, info, err := decodeScript(testSubject) - require.NoError(t, err) + require.Nil(t, err) require.Equal(t, info, "32;3;gzip=1") require.Equal(t, s, "ls\n") } diff --git a/internal/commandProcessor/commandProcessor.go b/internal/commandProcessor/commandProcessor.go index 19cab6e..daed652 100644 --- a/internal/commandProcessor/commandProcessor.go +++ b/internal/commandProcessor/commandProcessor.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-extension-platform/pkg/handlerenv" "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/internal/instanceview" @@ -22,6 +23,8 @@ import ( var ( handlerEnvironmentGetter func(name, version string) (he *handlerenv.HandlerEnvironment, _ error) = handlerenv.GetHandlerEnvironment + fnGetHandlerSettings = handlersettings.GetHandlerSettings + fnReportInstanceView = instanceview.ReportInstanceView ) func ProcessImmediateHandlerCommand(cmd types.Cmd, hs handlersettings.HandlerSettingsFile, extensionName string, seqNum int) error { @@ -71,17 +74,18 @@ func ProcessHandlerCommand(cmd types.Cmd) error { func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv types.HandlerEnvironment, extensionName string, seqNum int, downloadFolder string) error { ctx.Log("message", fmt.Sprintf("processing command for extensionName: %v and seqNum: %v", extensionName, seqNum)) instView := types.RunCommandInstanceView{ - ExecutionState: types.Running, - ExecutionMessage: "Execution in progress", - ExitCode: 0, - Output: "", - Error: "", - StartTime: time.Now().UTC().Format(time.RFC3339), - EndTime: "", + ExecutionState: types.Running, + ExecutionMessage: "Execution in progress", + ExitCode: 0, + Output: "", + Error: "", + StartTime: time.Now().UTC().Format(time.RFC3339), + EndTime: "", + ErrorClarificationValue: 0, } metadata := types.NewRCMetadata(extensionName, seqNum, downloadFolder, constants.DataDir) - instanceview.ReportInstanceView(ctx, hEnv, metadata, types.StatusTransitioning, cmd, &instView) + fnReportInstanceView(ctx, hEnv, metadata, types.StatusTransitioning, cmd, &instView) // execute the subcommand stdout, stderr, cmdInvokeError, exitCode := cmd.Functions.Invoke(ctx, hEnv, &instView, metadata, cmd) @@ -96,13 +100,24 @@ func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv type instView.ExitCode = exitCode statusToReport := types.StatusSuccess + // Add an error clarification if we have one + var ewc *vmextension.ErrorWithClarification + if errors.As(cmdInvokeError, &ewc) { + instView.ErrorClarificationValue = ewc.ErrorCode + } + // If TreatFailureAsDeploymentFailure is set to true and the exit code is non-zero, set extension status to error - cfg, err := handlersettings.GetHandlerSettings(hEnv.HandlerEnvironment.ConfigFolder, extensionName, seqNum, ctx) + cfg, err := fnGetHandlerSettings(hEnv.HandlerEnvironment.ConfigFolder, extensionName, seqNum, ctx) if err == nil && cfg.PublicSettings.TreatFailureAsDeploymentFailure && cmd.FailExitCode != 0 { statusToReport = types.StatusError } - instanceview.ReportInstanceView(ctx, hEnv, metadata, statusToReport, cmd, &instView) + fnReportInstanceView(ctx, hEnv, metadata, statusToReport, cmd, &instView) + + if err == nil { + return nil + } + return errors.Wrapf(err, "command execution failed") } else { // No error. Succeeded instView.ExecutionMessage = "Execution completed" @@ -111,7 +126,7 @@ func ProcessHandlerCommandWithDetails(ctx *log.Context, cmd types.Cmd, hEnv type instView.ExitCode = constants.ExitCode_Okay } - instanceview.ReportInstanceView(ctx, hEnv, metadata, types.StatusSuccess, cmd, &instView) + fnReportInstanceView(ctx, hEnv, metadata, types.StatusSuccess, cmd, &instView) ctx.Log("event", "end") return nil diff --git a/internal/commandProcessor/commandProcessor_test.go b/internal/commandProcessor/commandProcessor_test.go index 56d99bf..2fedbc7 100644 --- a/internal/commandProcessor/commandProcessor_test.go +++ b/internal/commandProcessor/commandProcessor_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/cleanup" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" @@ -190,3 +191,38 @@ func Test_GetSeqNumFromFailedToFind(t *testing.T) { require.Nil(t, err) require.Equal(t, 0, actualSeqNum) } + +func Test_ProcessHandlerCommandWithDetails_Failure_WithClarification(t *testing.T) { + var last types.RunCommandInstanceView + + origReportInstanceView := fnReportInstanceView + defer func() { fnReportInstanceView = origReportInstanceView }() + fnReportInstanceView = func(ctx *log.Context, hEnv types.HandlerEnvironment, metadata types.RCMetadata, t types.StatusType, c types.Cmd, instanceview *types.RunCommandInstanceView) error { + last = *instanceview + return nil + } + + mockFunc := types.CmdFunctions{ + Invoke: func(_ *log.Context, _ types.HandlerEnvironment, iv *types.RunCommandInstanceView, _ types.RCMetadata, _ types.Cmd) (string, string, error, int) { + return "x", "y", vmextension.NewErrorWithClarificationPtr(1234, errors.New("the chipmunks are upset")), 3 + }, + } + + orig := fnGetHandlerSettings + defer func() { fnGetHandlerSettings = orig }() + fnGetHandlerSettings = func(string, string, int, *log.Context) (handlersettings.HandlerSettings, *vmextension.ErrorWithClarification) { + return handlersettings.HandlerSettings{ + PublicSettings: handlersettings.PublicSettings{ + TreatFailureAsDeploymentFailure: false, + }, + }, nil + } + + cmd := types.Cmd{Name: "run", FailExitCode: 3, Functions: mockFunc} + hEnv := types.HandlerEnvironment{} + ctx := log.NewContext(log.NewNopLogger()) + + err := ProcessHandlerCommandWithDetails(ctx, cmd, hEnv, "x", 1, "/tmp") + require.Nil(t, err, "expected no error because TreatFailureAsDeploymentFailure is false") + require.Equal(t, 1234, last.ErrorClarificationValue, "expected clarification 1234 but got %d", last.ErrorClarificationValue) +} diff --git a/internal/constants/errorclarification.go b/internal/constants/errorclarification.go new file mode 100644 index 0000000..e70cd91 --- /dev/null +++ b/internal/constants/errorclarification.go @@ -0,0 +1,139 @@ +package constants + +const ( + FileDownload_BadRequest = -41 + FileDownload_UnknownError = -40 + FileDownload_StorageError = -42 + FileDownload_UnhandledError = -43 + FileDownload_StorageClientInitialization = -44 + FileDownload_CreateDirectoryFailure = -45 + FileDownload_OpenFileForWriteFailure = -46 + FileDownload_CouldNotCreateRequest = -47 + FileDownload_InternalServerError = -48 + FileDownload_WriteFileError = -49 + + Internal_CouldNotFindCertificate = -20 + Internal_CouldNotDecrypt = -22 + Internal_ArtifactCountMismatch = -23 + Internal_ArtifactDoesNotExist = -24 + Internal_IncorrectRunAsScriptPath = -25 + Internal_RunAsOpenSourceScriptFileFailed = -26 + Internal_RunAsCreateRunAsScriptFileFailed = -27 + Internal_RunAsCopySourceScriptToRunAsScriptFileFailed = -28 + Internal_RunAsLookupUserUidFailed = -29 + Internal_RunAsScriptFileChangeOwnerFailed = -30 + Internal_RunAsScriptFileChangePermissionsFailed = -31 + + Internal_CouldNotParseSettings = -32 + Internal_InvalidHandlerSettingsJson = -33 + Internal_InvalidHandlerSettingsCount = -34 + Internal_NoHandlerSettingsThumbprint = -35 + Internal_HandlerSettingsFailedToDecode = -36 + Internal_DecryptingProtectedSettingsFailed = -37 + Internal_UnmarshalProtectedSettingsFailed = -38 + Internal_UnmarshalSettingsFailed = -39 + Internal_UnmarshalPublicSettingsFailed = -40 + Internal_InvalidArtifactSpecification = -41 + + Internal_CouldNotCreateStatusDirectory = -60 + Internal_ExtensionDirectoryNameEmpty = -61 + Internal_CouldNotOpenSubdirectory = -62 + Internal_CouldNotReadDirectoryEntries = -63 + Internal_FailedToOpenFileForReading = -64 + Internal_FailedToCreateFile = -65 + Internal_FailedToCopyFile = -66 + Internal_FailedToReadFile = -67 + Internal_CouldNotOpenFileForWriting = -68 + + Immediate_CouldNotDetermineServiceInstalled = -70 + Immediate_CouldNotDetermineInstalledVersion = -71 + Immediate_CouldNotMarkBinaryAsExecutable = -72 + Immediate_CouldNotRemoveOldUnitConfigFile = -73 + Immediate_ErrorCreatingUnitConfig = -74 + Immediate_ErrorReloadingDaemonWorker = -75 + Immediate_ErrorEnablingUnit = -76 + Immediate_CouldNotStartService = -77 + Immediate_CouldNotCheckServiceAlreadyEnabled = -78 + Immediate_EnableServiceFailed = -79 + + Script_FailedToDecode = -101 + Script_FailedToDecompress = -102 + + Hgap_FailedCreateRequest = -120 + Hgap_CertificateMissingFromGoalState = -121 + Hgap_NoCertThumbprint = -122 + Hgap_FailedToCreateRequestFactory = -123 + Hgap_FailedToParseAddress = -124 + Hgap_EtagNotFound = -125 + Hgap_FailedToParseImmediateSettings = -126 + Hgap_CouldNotCreateRequestManager = -127 + Hgap_InternalArgumentError = -128 + + HandlerEnv_CouldNotFindBaseDirectory = -140 + HandlerEnv_HandlingError = -141 + HandlerEnv_NotFound = -142 + HandlerEnv_UnmarshalFailed = -143 + HandlerEnv_InvalidConfigCount = -144 + + Msi_CouldNotDeserializeResponse = -90 + + Internal_UnknownError = -200 + + SystemError = -1 // CRP will interpret anything > 0 as a user error + + // User errors + CommandExecution_BadConfig = 1 + CommandExecution_FailureExitCode = 2 + CommandExecution_TimedOut = 4 + CommandExecution_RunAsCreateProcessFailed = 5 + CommandExecution_RunAsUserLogonFailed = 6 + CommandExecution_CouldNotStart = 7 + + CustomerInput_StorageCredsAndMIBothSpecified = 26 + CustomerInput_ClientIdObjectIdBothSpecified = 27 + CustomerInput_ErrorAndOutputBlobsSame = 28 + CustomerInput_NoScriptSpecified = 29 + + FileDownload_AccessDenied = 52 + FileDownload_DoesNotExist = 53 + FileDownload_NetworkingError = 54 + FileDownload_GenericError = 55 + FileDownload_UnableToWriteFile = 57 + ArtifactDownload_GenericError = 58 + FileDownload_UnableToParseFileName = 59 + FileDownload_CannotExtractFileNameFromUrl = 60 + FileDownload_InvalidFileName = 61 + FileDownload_FailedStatusCode = 62 + FileDownload_CannotParseUrl = 63 + FileDownload_CannotGenerateSasKey = 64 + FileDownload_Empty = 65 + + Msi_NotFound = 70 + Msi_DoesNotHaveRightPermissions = 71 + Msi_GenericRetrievalError = 72 + + AppendBlobCreation_DoesNotExist = 90 + AppendBlobCreation_PermissionsIssue = 91 + AppendBlobCreation_Other = 92 + AppendBlobCreation_InvalidUri = 93 + AppendBlobCreation_InvalidMsi = 94 + AppendBlobCreation_ObjectIdNotSupported = 95 + AppendBlobCreation_ClientError = 96 + + ImmediateRC_ExceededConcurrentLimit = 100 + ImmediateRC_TaskCanceled = 101 + ImmediateRC_TaskTimeout = 102 + ImmediateRC_UnknownFailure = 103 + ImmediateRC_UnhandledException = 104 + ImmediateRC_CommandSkipped = 105 + + FileSystem_CreateDataDirectoryFailed = 110 + FileSystem_RemoveDataDirectoryFailed = 121 + FileSystem_OpenStandardOutFailed = 122 + FileSystem_OpenStandardErrorFailed = 123 + + Immediate_Systemd_NotSupported = 140 + + Http_RequestFailure = 150 + Http_FailedStatusCode = 151 +) diff --git a/internal/constants/exitcodes.go b/internal/constants/exitcodes.go index 7414ec6..5c20b2b 100755 --- a/internal/constants/exitcodes.go +++ b/internal/constants/exitcodes.go @@ -4,37 +4,11 @@ const ( // Exit codes ExitCode_Okay = 0 - // User errors (-100s): - ExitCode_ScriptBlobDownloadFailed = -100 - ExitCode_BlobCreateOrReplaceFailed = -101 - ExitCode_RunAsLookupUserFailed = -102 - // Service Errors (-200s): - ExitCode_CreateDataDirectoryFailed = -200 - ExitCode_RemoveDataDirectoryFailed = -201 - ExitCode_GetHandlerSettingsFailed = -202 - ExitCode_SaveScriptFailed = -203 - ExitCode_CommandExecutionFailed = -204 - ExitCode_OpenStdOutFileFailed = -205 - ExitCode_OpenStdErrFileFailed = -206 - ExitCode_IncorrectRunAsScriptPath = -207 - ExitCode_RunAsIncorrectScriptPath = -208 - ExitCode_RunAsOpenSourceScriptFileFailed = -209 - ExitCode_RunAsCreateRunAsScriptFileFailed = -210 - ExitCode_RunAsCopySourceScriptToRunAsScriptFileFailed = -211 - ExitCode_RunAsLookupUserUidFailed = -212 - ExitCode_RunAsScriptFileChangeOwnerFailed = -213 - ExitCode_RunAsScriptFileChangePermissionsFailed = -214 - ExitCode_DownloadArtifactFailed = -215 - ExitCode_UpgradeInstalledServiceFailed = -216 - ExitCode_InstallServiceFailed = -217 - ExitCode_UninstallInstalledServiceFailed = -218 - ExitCode_DisableInstalledServiceFailed = -219 - ExitCode_CopyStateForUpdateFailed = -220 - ExitCode_SkippedImmediateGoalState = -221 - ExitCode_ImmediateTaskTimeout = -222 - ExitCode_ImmediateTaskFailed = -223 - ExitCode_CouldNotRehydrateMrSeq = -224 - - // Unknown errors (-300s): + ExitCode_UpgradeInstalledServiceFailed = -216 + ExitCode_InstallServiceFailed = -217 + ExitCode_UninstallInstalledServiceFailed = -218 + ExitCode_DisableInstalledServiceFailed = -219 + ExitCode_CopyStateForUpdateFailed = -220 + ExitCode_CouldNotRehydrateMrSeq = -224 ) diff --git a/internal/exec/exec.go b/internal/exec/exec.go index 5678159..ee2af89 100644 --- a/internal/exec/exec.go +++ b/internal/exec/exec.go @@ -13,18 +13,31 @@ import ( "syscall" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/go-kit/kit/log" "github.com/pkg/errors" ) +var ( + fnIoCopy = io.Copy + fnOsChMod = os.Chmod + fnOsChown = os.Chown + fnOsCreate = os.Create + fnOsMkDirAll = os.MkdirAll + fnOsOpenFile = os.OpenFile + fnOsSetEnv = os.Setenv + FnRunCommand = runCommand + fnUserLookup = user.Lookup +) + // Exec runs the given cmd in /bin/sh, saves its stdout/stderr streams to // the specified files. It waits until the execution terminates. // // On error, an exit code may be returned if it is an exit code error. // Given stdout and stderr will be closed upon returning. -func Exec(ctx *log.Context, cmd, workdir string, stdout, stderr io.WriteCloser, cfg *handlersettings.HandlerSettings) (int, error) { +func Exec(ctx *log.Context, cmd, workdir string, stdout, stderr io.WriteCloser, cfg *handlersettings.HandlerSettings) (int, *vmextension.ErrorWithClarification) { defer stdout.Close() defer stderr.Close() @@ -43,7 +56,7 @@ func Exec(ctx *log.Context, cmd, workdir string, stdout, stderr io.WriteCloser, if !strings.HasPrefix(scriptPath, constants.DataDir) { errMessage := "Failed to determine RunAs script path. Contact ICM team AzureRT\\Extensions for this service error." ctx.Log("message", errMessage) - return constants.ExitCode_RunAsIncorrectScriptPath, errors.New(errMessage) + return constants.Internal_IncorrectRunAsScriptPath, vmextension.NewErrorWithClarificationPtr(constants.Internal_IncorrectRunAsScriptPath, errors.New(errMessage)) } // Gets suffix "download//0/script.sh" @@ -53,59 +66,59 @@ func Exec(ctx *log.Context, cmd, workdir string, stdout, stderr io.WriteCloser, runAsScriptDirectoryPath := filepath.Dir(runAsScriptFilePath) // Get directory of runAsScript that doesn't exist yet // Create runAsScriptDirectoryPath and its intermediate directories if they do not exist - os.MkdirAll(runAsScriptDirectoryPath, 0777) + fnOsMkDirAll(runAsScriptDirectoryPath, 0777) /// Copy source script at scriptPath to runAsScriptDirectoryPath // Get reference to source script by opening it - sourceScriptFile, sourceScriptFileOpenError := os.OpenFile(scriptPath, os.O_RDONLY, 0400) + sourceScriptFile, sourceScriptFileOpenError := fnOsOpenFile(scriptPath, os.O_RDONLY, 0400) if sourceScriptFileOpenError != nil { - errMessage := "Failed to open source script. Contact ICM team AzureRT\\Extensions for this service error." - ctx.Log("message", errMessage+fmt.Sprintf(" Source script file is '%s'", scriptPath)) - return constants.ExitCode_RunAsOpenSourceScriptFileFailed, errors.Wrapf(sourceScriptFileOpenError, errMessage) + errMessage := fmt.Sprintf("Failed to open source script. Contact ICM team AzureRT\\Extensions for this service error. Source script file is '%s'", scriptPath) + ctx.Log("message", errMessage) + return constants.Internal_RunAsOpenSourceScriptFileFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsOpenSourceScriptFileFailed, sourceScriptFileOpenError) } - destScriptFile, destScriptCreateError := os.Create(runAsScriptFilePath) + destScriptFile, destScriptCreateError := fnOsCreate(runAsScriptFilePath) if destScriptCreateError != nil { - errMessage := "Failed to create script for Run As in Run As directory. Contact ICM team AzureRT\\Extensions for this service error." - ctx.Log("message", errMessage+fmt.Sprintf(" Destination runAs script file is '%s'", runAsScriptFilePath)) - return constants.ExitCode_RunAsCreateRunAsScriptFileFailed, errors.Wrapf(destScriptCreateError, errMessage) + errMessage := fmt.Sprintf("Failed to create script for Run As in Run As directory. Contact ICM team AzureRT\\Extensions for this service error. Destination runAs script file is '%s'", runAsScriptFilePath) + ctx.Log("message", errMessage) + return constants.Internal_RunAsOpenSourceScriptFileFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsOpenSourceScriptFileFailed, destScriptCreateError) } - _, runAsScriptCopyError := io.Copy(destScriptFile, sourceScriptFile) + _, runAsScriptCopyError := fnIoCopy(destScriptFile, sourceScriptFile) if runAsScriptCopyError != nil { errMessage := fmt.Sprintf("Failed to copy script file '%s' to Run As path '%s'. Contact ICM team AzureRT\\Extensions for this service error.", scriptPath, runAsScriptFilePath) ctx.Log("message", errMessage) - return constants.ExitCode_RunAsCopySourceScriptToRunAsScriptFileFailed, errors.Wrapf(runAsScriptCopyError, errMessage) + return constants.Internal_RunAsCopySourceScriptToRunAsScriptFileFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsCopySourceScriptToRunAsScriptFileFailed, runAsScriptCopyError) } sourceScriptFile.Close() destScriptFile.Close() // Provide read and execute permissions to RunAsUser on .sh file at runAsScriptFilePath - lookedUpUser, lookupUserError := user.Lookup(cfg.PublicSettings.RunAsUser) + lookedUpUser, lookupUserError := fnUserLookup(cfg.PublicSettings.RunAsUser) if lookupUserError != nil { errMessage := fmt.Sprintf("Failed to lookup RunAs user '%s'. Looks like user does not exist. For RunAs to work properly, contact admin of VM and make sure RunAs user is added on the VM and user has access to resources accessed by the Run Command (Directories, Files, Network etc.). Refer: https://aka.ms/RunCommandManagedLinux", cfg.PublicSettings.RunAsUser) ctx.Log("message", errMessage) - return constants.ExitCode_RunAsLookupUserFailed, errors.Wrapf(lookupUserError, errMessage) + return constants.CommandExecution_RunAsUserLogonFailed, vmextension.NewErrorWithClarificationPtr(constants.CommandExecution_RunAsUserLogonFailed, lookupUserError) } lookedUpUserUid, lookedUpUserUidErr := strconv.Atoi(lookedUpUser.Uid) if lookedUpUserUidErr != nil { errMessage := "Failed to determine RunAs user's Uid and Guid . Contact ICM team AzureRT\\Extensions for this service error." ctx.Log("message", errMessage) - return constants.ExitCode_RunAsLookupUserUidFailed, errors.Wrapf(lookedUpUserUidErr, errMessage) + return constants.Internal_RunAsLookupUserUidFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsLookupUserUidFailed, lookedUpUserUidErr) } - runAsScriptChownError := os.Chown(runAsScriptFilePath, lookedUpUserUid, os.Getegid()) + runAsScriptChownError := fnOsChown(runAsScriptFilePath, lookedUpUserUid, os.Getegid()) if runAsScriptChownError != nil { errMessage := fmt.Sprintf("Failed to change owner of file '%s' to RunAs user '%s'. Contact ICM team AzureRT\\Extensions for this service error.", runAsScriptFilePath, cfg.PublicSettings.RunAsUser) ctx.Log("message", errMessage) - return constants.ExitCode_RunAsScriptFileChangeOwnerFailed, errors.Wrapf(runAsScriptChownError, errMessage) + return constants.Internal_RunAsScriptFileChangeOwnerFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsScriptFileChangeOwnerFailed, runAsScriptChownError) } - runAsScriptChmodError := os.Chmod(runAsScriptFilePath, 0550) + runAsScriptChmodError := fnOsChMod(runAsScriptFilePath, 0550) if runAsScriptChmodError != nil { errMessage := fmt.Sprintf("Failed to change permissions to execute for file '%s' for RunAs user '%s'. Contact ICM team AzureRT\\Extensions for this service error.", runAsScriptFilePath, cfg.PublicSettings.RunAsUser) ctx.Log("message", errMessage) - return constants.ExitCode_RunAsScriptFileChangePermissionsFailed, errors.Wrapf(runAsScriptChmodError, errMessage) + return constants.Internal_RunAsScriptFileChangePermissionsFailed, vmextension.NewErrorWithClarificationPtr(constants.Internal_RunAsScriptFileChangePermissionsFailed, runAsScriptChmodError) } // echo pipes the RunAsPassword to sudo -S for RunAsUser instead of prompting the password interactively from user and blocking. @@ -127,21 +140,39 @@ func Exec(ctx *log.Context, cmd, workdir string, stdout, stderr io.WriteCloser, command.Dir = workdir command.Stdout = stdout command.Stderr = stderr - err = command.Run() + err = FnRunCommand(command) if err != nil { exitErr, ok := err.(*exec.ExitError) if ok { if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { exitCode = status.ExitStatus() + commandExitCode := exitCode if status.Signaled() { // Timed out ctx.Log("message", "Timeout:"+err.Error()) + exitCode = constants.CommandExecution_TimedOut + } else if exitCode != 0 { + exitCode = constants.CommandExecution_FailureExitCode } - return exitCode, fmt.Errorf("command terminated with exit status=%d", exitCode) + + commandFailedErr := fmt.Errorf("command terminated with exit status=%d", commandExitCode) + return exitCode, vmextension.NewErrorWithClarificationPtr(exitCode, commandFailedErr) + } + } else { + startErr, ok := err.(*exec.Error) + if ok { + exitCode = constants.CommandExecution_CouldNotStart + commandFailedErr := fmt.Errorf("Command failed to start with error=%s", startErr) + return exitCode, vmextension.NewErrorWithClarificationPtr(exitCode, commandFailedErr) } } } - return exitCode, errors.Wrapf(err, "failed to execute command") + // The command succeeded + return exitCode, nil +} + +func runCommand(command *exec.Cmd) error { + return command.Run() } func SetEnvironmentVariables(cfg *handlersettings.HandlerSettings) (string, error) { @@ -160,7 +191,7 @@ func SetEnvironmentVariables(cfg *handlersettings.HandlerSettings) (string, erro value := parameters[i].Value if value != "" { if name != "" { // Named parameters are set as environmental setting - err = os.Setenv(name, value) + err = fnOsSetEnv(name, value) } else { // Unnamed parameters go to command args commandArgs += " " + value } @@ -176,21 +207,21 @@ func SetEnvironmentVariables(cfg *handlersettings.HandlerSettings) (string, erro // // Ideally, we execute commands only once per sequence number in run-command-handler, // and save their output under /var/lib/waagent//download//*. -func ExecCmdInDir(ctx *log.Context, scriptFilePath, workdir string, cfg *handlersettings.HandlerSettings) (error, int) { +func ExecCmdInDir(ctx *log.Context, scriptFilePath, workdir string, cfg *handlersettings.HandlerSettings) (*vmextension.ErrorWithClarification, int) { stdoutFileName, stderrFileName := LogPaths(workdir) - outF, err := os.OpenFile(stdoutFileName, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + outF, err := fnOsOpenFile(stdoutFileName, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) if err != nil { - return errors.Wrapf(err, "failed to open stdout file"), constants.ExitCode_OpenStdOutFileFailed + return vmextension.NewErrorWithClarificationPtr(constants.FileSystem_OpenStandardOutFailed, fmt.Errorf("failed to open stdout file: %v", err)), constants.FileSystem_OpenStandardOutFailed } - errF, err := os.OpenFile(stderrFileName, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) + errF, err := fnOsOpenFile(stderrFileName, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600) if err != nil { - return errors.Wrapf(err, "failed to open stderr file"), constants.ExitCode_OpenStdErrFileFailed + return vmextension.NewErrorWithClarificationPtr(constants.FileSystem_OpenStandardErrorFailed, fmt.Errorf("failed to open stderr file: %v", err)), constants.FileSystem_OpenStandardErrorFailed } - exitCode, err := Exec(ctx, scriptFilePath, workdir, outF, errF, cfg) - return err, exitCode + exitCode, ewc := Exec(ctx, scriptFilePath, workdir, outF, errF, cfg) + return ewc, exitCode } // LogPaths returns stdout and stderr file paths for the specified output diff --git a/internal/exec/exec_test.go b/internal/exec/exec_test.go index 14fb4cf..3ff575f 100644 --- a/internal/exec/exec_test.go +++ b/internal/exec/exec_test.go @@ -3,11 +3,16 @@ package exec import ( "bytes" "errors" + "io" "io/ioutil" "os" + "os/exec" + "os/user" "path/filepath" + "strings" "testing" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/go-kit/kit/log" @@ -19,24 +24,310 @@ var ( testContext = log.NewContext(log.NewNopLogger()) ) -func TestExec_success(t *testing.T) { - v := new(mockFile) - ec, err := Exec(testContext, "date", "/", v, v, &testHandlerSettings) - require.Nil(t, err, "err: %v -- out: %s", err, v.b.Bytes()) - require.EqualValues(t, 0, ec) +func TestExec_SuccessExitCodeOkay(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + out := newCloseRecorder() + errw := newCloseRecorder() + + exitCode, err := Exec(newCtx(), "echo hi", t.TempDir(), out, errw, cfg) + require.Nil(t, err) + require.Equal(t, constants.ExitCode_Okay, exitCode) + require.Contains(t, out.String(), "hi") } -func TestExec_success_redirectsStdStreams_closesFds(t *testing.T) { - o, e := new(mockFile), new(mockFile) - require.False(t, o.closed, "stdout open") - require.False(t, e.closed, "stderr open") +func TestExec_AlwaysClosesStreams(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() - _, err := Exec(testContext, "/bin/echo 'I am stdout!'>&1; /bin/echo 'I am stderr!'>&2", "/", o, e, &testHandlerSettings) - require.Nil(t, err, "err: %v -- stderr: %s", err, e.b.Bytes()) - require.Equal(t, "I am stdout!\n", string(o.b.Bytes())) - require.Equal(t, "I am stderr!\n", string(e.b.Bytes())) - require.True(t, o.closed, "stdout closed") - require.True(t, e.closed, "stderr closed") + cfg := minimalCfg() + out := newCloseRecorder() + errw := newCloseRecorder() + + // Force runCommand to return error to ensure closures happen on error path. + FnRunCommand = func(_ *exec.Cmd) error { + return errors.New("the chipmunks have revolted") + } + + _, _ = Exec(newCtx(), "/bin/true", t.TempDir(), out, errw, cfg) + require.True(t, out.closed, "stdout should be closed") + require.True(t, errw.closed, "stderr should be closed") +} + +func TestExec_RunAsUser_InvalidScriptPathPrefix(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + out := newCloseRecorder() + errw := newCloseRecorder() + + // Script path does NOT start with constants.DataDir => should fail early + exitCode, err := Exec(newCtx(), "/tmp/not-under-datadir/script.sh", t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_IncorrectRunAsScriptPath, exitCode) + VerifyErrorClarification(t, constants.Internal_IncorrectRunAsScriptPath, err) +} + +func TestExec_RunAsUser_OpenSourceScriptFails_ReturnsOpenSourceFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + // Fail opening the source script + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { + return nil, errors.New("open failed") + } + // Ensure we don't accidentally reach execution + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsOpenSourceScriptFileFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsOpenSourceScriptFileFailed, err) +} + +func TestExec_RunAsUser_CreateDestScriptFails_ReturnsOpenSourceFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + // Open source succeeds + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { + return nil, nil // File won't be used before the method fails + } + // Dest create fails + fnOsCreate = func(_ string) (*os.File, error) { + return nil, errors.New("create failed") + } + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsOpenSourceScriptFileFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsOpenSourceScriptFileFailed, err) +} + +func TestExec_RunAsUser_CopyFails_ReturnsCopyFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { + return nil, nil // File won't be used before we fail + } + + fnOsCreate = func(_ string) (*os.File, error) { + return nil, nil + } + + // Copy fails + fnIoCopy = func(_ io.Writer, _ io.Reader) (int64, error) { + return 0, errors.New("the chipmunks do not copy") + } + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsCopySourceScriptToRunAsScriptFileFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsCopySourceScriptToRunAsScriptFileFailed, err) +} + +func TestExec_RunAsUser_LookupUserFails_ReturnsRunAsUserLogonFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + // Open + create + copy succeed + tmpSrc := filepath.Join(t.TempDir(), "src.sh") + require.NoError(t, os.WriteFile(tmpSrc, []byte("echo hi\n"), 0600)) + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { return os.Open(tmpSrc) } + fnOsCreate = func(_ string) (*os.File, error) { return os.CreateTemp(t.TempDir(), "dest-*") } + fnIoCopy = func(_ io.Writer, _ io.Reader) (int64, error) { return 1, nil } + + // User lookup fails + fnUserLookup = func(_ string) (*user.User, error) { + return nil, errors.New("no such chipmunk") + } + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.CommandExecution_RunAsUserLogonFailed, code) + VerifyErrorClarification(t, constants.CommandExecution_RunAsUserLogonFailed, err) +} + +func TestExec_RunAsUser_UidParseFails_ReturnsLookupUserUidFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + tmpSrc := filepath.Join(t.TempDir(), "src.sh") + require.NoError(t, os.WriteFile(tmpSrc, []byte("echo hi\n"), 0600)) + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { return os.Open(tmpSrc) } + fnOsCreate = func(_ string) (*os.File, error) { return os.CreateTemp(t.TempDir(), "dest-*") } + fnIoCopy = func(_ io.Writer, _ io.Reader) (int64, error) { return 1, nil } + + // Lookup returns non-int Uid + fnUserLookup = func(_ string) (*user.User, error) { + return &user.User{Uid: "not-an-int"}, nil + } + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsLookupUserUidFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsLookupUserUidFailed, err) +} + +func TestExec_RunAsUser_ChownFails_ReturnsChangeOwnerFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + tmpSrc := filepath.Join(t.TempDir(), "src.sh") + require.NoError(t, os.WriteFile(tmpSrc, []byte("echo hi\n"), 0600)) + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { return os.Open(tmpSrc) } + fnOsCreate = func(_ string) (*os.File, error) { return os.CreateTemp(t.TempDir(), "dest-*") } + fnIoCopy = func(_ io.Writer, _ io.Reader) (int64, error) { return 1, nil } + + fnUserLookup = func(_ string) (*user.User, error) { + return &user.User{Uid: "1234"}, nil + } + fnOsChown = func(_ string, _ int, _ int) error { + return errors.New("chown failed") + } + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsScriptFileChangeOwnerFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsScriptFileChangeOwnerFailed, err) +} + +func TestExec_RunAsUser_ChmodFails_ReturnsChangePermissionsFailed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.RunAsUser = "someuser" + + tmpSrc := filepath.Join(t.TempDir(), "src.sh") + require.NoError(t, os.WriteFile(tmpSrc, []byte("echo hi\n"), 0600)) + fnOsOpenFile = func(_ string, _ int, _ os.FileMode) (*os.File, error) { return os.Open(tmpSrc) } + fnOsCreate = func(_ string) (*os.File, error) { return os.CreateTemp(t.TempDir(), "dest-*") } + fnIoCopy = func(_ io.Writer, _ io.Reader) (int64, error) { return 1, nil } + + fnUserLookup = func(_ string) (*user.User, error) { + return &user.User{Uid: "1234"}, nil + } + fnOsChown = func(_ string, _ int, _ int) error { return nil } + fnOsChMod = func(_ string, _ os.FileMode) error { + return errors.New("chmod failed") + } + + FnRunCommand = func(_ *exec.Cmd) error { + t.Fatalf("fnRunCommand should not be called when RunAs setup fails") + return nil + } + + out := newCloseRecorder() + errw := newCloseRecorder() + + code, err := Exec(newCtx(), constants.DataDir, t.TempDir(), out, errw, cfg) + require.Error(t, err) + require.Equal(t, constants.Internal_RunAsScriptFileChangePermissionsFailed, code) + VerifyErrorClarification(t, constants.Internal_RunAsScriptFileChangePermissionsFailed, err) +} + +func TestExecCmdInDir_OpenStdoutFails(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + + fnOsOpenFile = func(name string, flag int, perm os.FileMode) (*os.File, error) { + if strings.HasSuffix(name, "stdout") { + return nil, errors.New("open stdout failed") + } + return os.CreateTemp(t.TempDir(), "stderr-*") + } + + err, code := ExecCmdInDir(newCtx(), "echo hi", t.TempDir(), cfg) + require.Error(t, err) + require.Equal(t, constants.FileSystem_OpenStandardOutFailed, code) + VerifyErrorClarification(t, constants.FileSystem_OpenStandardOutFailed, err) +} + +func TestExecCmdInDir_OpenStderrFails(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + + fnOsOpenFile = func(name string, flag int, perm os.FileMode) (*os.File, error) { + if strings.HasSuffix(name, "stdout") { + return nil, nil // The directory won't be used before stderr fails + } + return nil, errors.New("open stderr failed") + } + + err, code := ExecCmdInDir(newCtx(), "echo hi", t.TempDir(), cfg) + require.Error(t, err) + require.Equal(t, constants.FileSystem_OpenStandardErrorFailed, code) + VerifyErrorClarification(t, constants.FileSystem_OpenStandardErrorFailed, err) } func TestExec_failure_exitError(t *testing.T) { @@ -55,41 +346,57 @@ func TestExec_failure_timeout(t *testing.T) { require.EqualValues(t, -1, ec) } -// func TestExec_runasuser(t *testing.T) { -// if os.Geteuid() != 0 { -// fmt.Println("SKIP: Should be run under root. Use sudo.") -// return -// } +func TestSetEnvironmentVariables_NamedAndUnnamed(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() -// o, e := new(mockFile), new(mockFile) -// testHandlerSettings.publicSettings.RunAsUser = "runcommand" -// exitCode, err := Exec(testContext, "whoami", "/", o, e, &testHandlerSettings) -// testHandlerSettings.publicSettings.RunAsUser = "" -// require.Nil(t, err) -// require.EqualValues(t, 0, exitCode) -// require.Equal(t, "runcommand\n", string(o.b.Bytes())) -// } + cfg := minimalCfg() + cfg.PublicSettings.Parameters = []handlersettings.ParameterDefinition{ + {Name: "FOO", Value: "bar"}, // named -> env + {Name: "", Value: "arg1"}, // unnamed -> arg + } + cfg.ProtectedSettings.ProtectedParameters = []handlersettings.ParameterDefinition{ + {Name: "BAZ", Value: "qux"}, + {Name: "", Value: "arg2"}, + } -func TestExec_SetEnvironmentVariables(t *testing.T) { - cfg := handlersettings.HandlerSettings{ - PublicSettings: handlersettings.PublicSettings{ - Parameters: []handlersettings.ParameterDefinition{ - {Name: "Variable1", Value: "value1"}, - {Name: "", Value: "arg1"}, - }, - }, - ProtectedSettings: handlersettings.ProtectedSettings{ - ProtectedParameters: []handlersettings.ParameterDefinition{ - {Name: "Variable2", Value: "value2"}, - {Name: "", Value: "arg2"}, - }, - }, + setCalls := map[string]string{} + fnOsSetEnv = func(k, v string) error { + setCalls[k] = v + return nil } - commandArgs, err := SetEnvironmentVariables(&cfg) - require.Nil(t, err) - require.Equal(t, commandArgs, " arg1 arg2") - require.Equal(t, "value1", os.Getenv("Variable1")) - require.Equal(t, "value2", os.Getenv("Variable2")) + + args, err := SetEnvironmentVariables(cfg) + require.NoError(t, err) + require.Contains(t, args, " arg1") + require.Contains(t, args, " arg2") + require.Equal(t, "bar", setCalls["FOO"]) + require.Equal(t, "qux", setCalls["BAZ"]) +} + +func TestSetEnvironmentVariables_ReturnsLastSetEnvError(t *testing.T) { + restore := saveAndRestoreFns() + defer restore() + + cfg := minimalCfg() + cfg.PublicSettings.Parameters = []handlersettings.ParameterDefinition{ + {Name: "X", Value: "1"}, + {Name: "Y", Value: "2"}, + } + + var call int + wantErr := errors.New("setenv failed") + fnOsSetEnv = func(k, v string) error { + call++ + if call == 2 { + return wantErr + } + return nil + } + + _, err := SetEnvironmentVariables(cfg) + require.Error(t, err) + require.Equal(t, wantErr, err) } func TestExec_failure_genericError(t *testing.T) { @@ -176,6 +483,78 @@ func Test_logPaths(t *testing.T) { // Test utilities +type closeRecorder struct { + buf *bytes.Buffer + closed bool +} + +func newCloseRecorder() *closeRecorder { + return &closeRecorder{buf: &bytes.Buffer{}} +} + +func (c *closeRecorder) Write(p []byte) (int, error) { return c.buf.Write(p) } +func (c *closeRecorder) Close() error { + c.closed = true + return nil +} +func (c *closeRecorder) String() string { return c.buf.String() } + +func newCtx() *log.Context { + return log.NewContext(log.NewNopLogger()) +} + +func minimalCfg() *handlersettings.HandlerSettings { + return &handlersettings.HandlerSettings{ + PublicSettings: handlersettings.PublicSettings{ + RunAsUser: "", + TimeoutInSeconds: 0, + Parameters: nil, + TreatFailureAsDeploymentFailure: false, + }, + ProtectedSettings: handlersettings.ProtectedSettings{ + RunAsPassword: "", + ProtectedParameters: nil, + }, + } +} + +type savedFns struct { + ioCopy func(dst io.Writer, src io.Reader) (written int64, err error) + chmod func(string, os.FileMode) error + chown func(string, int, int) error + create func(string) (*os.File, error) + mkdirAll func(string, os.FileMode) error + openFile func(string, int, os.FileMode) (*os.File, error) + setEnv func(string, string) error + runCommand func(*exec.Cmd) error + userLookup func(string) (*user.User, error) +} + +func saveAndRestoreFns() func() { + s := savedFns{ + ioCopy: fnIoCopy, + chmod: fnOsChMod, + chown: fnOsChown, + create: fnOsCreate, + mkdirAll: fnOsMkDirAll, + openFile: fnOsOpenFile, + setEnv: fnOsSetEnv, + runCommand: FnRunCommand, + userLookup: fnUserLookup, + } + return func() { + fnIoCopy = s.ioCopy + fnOsChMod = s.chmod + fnOsChown = s.chown + fnOsCreate = s.create + fnOsMkDirAll = s.mkdirAll + fnOsOpenFile = s.openFile + fnOsSetEnv = s.setEnv + FnRunCommand = s.runCommand + fnUserLookup = s.userLookup + } +} + type mockFile struct { b bytes.Buffer closed bool @@ -204,3 +583,8 @@ func fileExists(t *testing.T, path string) bool { t.Fatalf("failed to check if %s exists: %v", path, err) return false } + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/internal/files/files.go b/internal/files/files.go index a1519cf..69d19da 100644 --- a/internal/files/files.go +++ b/internal/files/files.go @@ -9,6 +9,8 @@ import ( "os" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/pkg/download" "github.com/Azure/run-command-handler-linux/pkg/preprocess" @@ -19,7 +21,7 @@ import ( var UseMockSASDownloadFailure bool = false -func DownloadAndProcessArtifact(ctx *log.Context, downloadDir string, artifact *handlersettings.UnifiedArtifact) (string, error) { +func DownloadAndProcessArtifact(ctx *log.Context, downloadDir string, artifact *handlersettings.UnifiedArtifact) (string, *vmextension.ErrorWithClarification) { fileName := artifact.FileName if fileName == "" { fileName = fmt.Sprintf("%s%d", "Artifact", artifact.ArtifactId) @@ -29,7 +31,7 @@ func DownloadAndProcessArtifact(ctx *log.Context, downloadDir string, artifact * return targetFilePath, err } -func DownloadAndProcessScript(ctx *log.Context, url, downloadDir string, cfg *handlersettings.HandlerSettings) (string, error) { +func DownloadAndProcessScript(ctx *log.Context, url, downloadDir string, cfg *handlersettings.HandlerSettings) (string, *vmextension.ErrorWithClarification) { fileName, err := UrlToFileName(url) if err != nil { return "", err @@ -45,19 +47,19 @@ func DownloadAndProcessScript(ctx *log.Context, url, downloadDir string, cfg *ha // downloadAndProcessURL downloads using the specified downloader and saves it to the // specified existing directory, which must be the path to the saved file. Then // it post-processes file based on heuristics. -func downloadAndProcessURL(ctx *log.Context, url, downloadDir string, fileName string, scriptSAS string, sourceManagedIdentity *handlersettings.RunCommandManagedIdentity) (string, error) { - var err error +func downloadAndProcessURL(ctx *log.Context, url, downloadDir string, fileName string, scriptSAS string, sourceManagedIdentity *handlersettings.RunCommandManagedIdentity) (string, *vmextension.ErrorWithClarification) { + var err *vmextension.ErrorWithClarification if !urlutil.IsValidUrl(url) { - return "", fmt.Errorf(url + " is not a valid url") // url does not contain SAS to se can log it + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CannotExtractFileNameFromUrl, fmt.Errorf(url+" is not a valid url")) } targetFilePath := filepath.Join(downloadDir, fileName) - var scriptSASDownloadErr error = nil + var scriptSASDownloadErr *vmextension.ErrorWithClarification = nil var downloadedFilePath string = "" if scriptSAS != "" { if UseMockSASDownloadFailure { - scriptSASDownloadErr = errors.New("Downloading script using SAS token failed.") + scriptSASDownloadErr = vmextension.NewErrorWithClarificationPtr(42, errors.New("Downloading script using SAS token failed.")) } else { downloadedFilePath, scriptSASDownloadErr = download.GetSASBlob(url, scriptSAS, downloadDir) } @@ -74,7 +76,7 @@ func downloadAndProcessURL(ctx *log.Context, url, downloadDir string, fileName s const mode = 0500 // we assume users download scripts to execute _, err = download.SaveTo(ctx, downloaders, targetFilePath, mode) } else { - return "", getDownloadersError + return "", vmextension.NewErrorWithClarificationPtr(constants.Msi_GenericRetrievalError, getDownloadersError) } } @@ -84,7 +86,7 @@ func downloadAndProcessURL(ctx *log.Context, url, downloadDir string, fileName s err = PostProcessFile(targetFilePath) if err != nil { - return "", errors.Wrapf(err, "failed to post-process '%s'", fileName) + return "", err } return targetFilePath, nil @@ -93,10 +95,10 @@ func downloadAndProcessURL(ctx *log.Context, url, downloadDir string, fileName s // getDownloaders returns one or two downloaders (two if it is an Azure storage blob): // 1. Downloader for script using public URI. // 2. Downloader for script using managed identity. -func getDownloaders(fileURL string, managedIdentity *handlersettings.RunCommandManagedIdentity, msiDownloader download.MsiDownloader) ([]download.Downloader, error) { +func getDownloaders(fileURL string, managedIdentity *handlersettings.RunCommandManagedIdentity, msiDownloader download.MsiDownloader) ([]download.Downloader, *vmextension.ErrorWithClarification) { if fileURL == "" { - return nil, fmt.Errorf("fileURL is empty") + return nil, vmextension.NewErrorWithClarificationPtr(constants.FileDownload_Empty, fmt.Errorf("fileURL is empty")) } if download.IsAzureStorageBlobUri(fileURL) { @@ -115,7 +117,7 @@ func getDownloaders(fileURL string, managedIdentity *handlersettings.RunCommandM // uses user-managed identity msiProvider = msiDownloader.GetMsiProviderByObjectId(fileURL, managedIdentity.ObjectId) default: - return nil, fmt.Errorf("use either ClientId or ObjectId for managed identity. Not both") + return nil, vmextension.NewErrorWithClarificationPtr(constants.CustomerInput_ClientIdObjectIdBothSpecified, fmt.Errorf("use either ClientId or ObjectId for managed identity. Not both")) } _, msiError := msiProvider() @@ -140,10 +142,10 @@ func getDownloaders(fileURL string, managedIdentity *handlersettings.RunCommandM // UrlToFileName parses given URL and returns the section after the last slash // character of the path segment to be used as a file name. If a value is not // found, an error is returned. -func UrlToFileName(fileURL string) (string, error) { +func UrlToFileName(fileURL string) (string, *vmextension.ErrorWithClarification) { u, err := url.Parse(fileURL) if err != nil { - return "", errors.Wrapf(err, "unable to parse URL: %q", fileURL) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_UnableToParseFileName, errors.Wrapf(err, "unable to parse URL: %q", fileURL)) } s := strings.Split(u.Path, "/") @@ -153,16 +155,16 @@ func UrlToFileName(fileURL string) (string, error) { return fn, nil } } - return "", fmt.Errorf("cannot extract file name from URL: %q", fileURL) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CannotExtractFileNameFromUrl, fmt.Errorf("cannot extract file name from URL: %q", fileURL)) } // postProcessFile determines if path is a script file based on heuristics // and makes in-place changes to the file with some post-processing such as BOM // and DOS-line endings fixes to make the script POSIX-friendly. -func PostProcessFile(path string) error { - ok, err := preprocess.IsTextFile(path) - if err != nil { - return errors.Wrapf(err, "error determining if script is a text file") +func PostProcessFile(path string) *vmextension.ErrorWithClarification { + ok, ewc := preprocess.IsTextFile(path) + if ewc != nil { + return ewc } if !ok { return nil @@ -170,22 +172,31 @@ func PostProcessFile(path string) error { b, err := ioutil.ReadFile(path) // read the file into memory for processing if err != nil { - return errors.Wrapf(err, "error reading file") + return vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToReadFile, errors.Wrapf(err, "error reading file")) } b = preprocess.RemoveBOM(b) b = preprocess.Dos2Unix(b) err = ioutil.WriteFile(path, b, 0) - return errors.Wrap(os.Rename(path, path), "error writing file") + + if err != nil { + return vmextension.NewErrorWithClarificationPtr(constants.FileDownload_WriteFileError, errors.Wrap(os.Rename(path, path), "error writing file")) + } + return nil } -func SaveScriptFile(filePath string, content string) error { +func SaveScriptFile(filePath string, content string) *vmextension.ErrorWithClarification { const mode = 0500 // scripts should have execute permissions file, err := os.OpenFile(filePath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode) if err != nil { - return errors.Wrap(err, "failed to open file for writing: "+filePath) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_CouldNotOpenFileForWriting, errors.Wrap(err, "failed to open file for writing: "+filePath)) } _, err = file.WriteString(content) file.Close() - return errors.Wrap(err, "failed to write to the file: "+filePath) + + if err != nil { + return vmextension.NewErrorWithClarificationPtr(constants.FileDownload_WriteFileError, errors.Wrap(err, "failed to write to the file: "+filePath)) + } + + return nil } diff --git a/internal/files/files_test.go b/internal/files/files_test.go index 2466a1e..d1c1bf7 100644 --- a/internal/files/files_test.go +++ b/internal/files/files_test.go @@ -8,6 +8,9 @@ import ( "path/filepath" "testing" + "github.com/Azure/azure-extension-foundation/msi" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/pkg/download" "github.com/ahmetalpbalkan/go-httpbin" @@ -98,6 +101,7 @@ func Test_urlToFileName_badURL(t *testing.T) { _, err := UrlToFileName("http://192.168.0.%31/") require.NotNil(t, err) require.Contains(t, err.Error(), `unable to parse URL: "http://192.168.0.%31/"`) + VerifyErrorClarification(t, constants.FileDownload_UnableToParseFileName, err) } func Test_urlToFileName_noFileName(t *testing.T) { @@ -117,6 +121,7 @@ func Test_urlToFileName_noFileName(t *testing.T) { _, err := UrlToFileName(c) require.NotNil(t, err, "not failed: %s", "url=%s", c) require.Contains(t, err.Error(), "cannot extract file name from URL", "url=%s", c) + VerifyErrorClarification(t, constants.FileDownload_CannotExtractFileNameFromUrl, err) } } @@ -136,7 +141,8 @@ func Test_urlToFileName(t *testing.T) { } func Test_postProcessFile_fail(t *testing.T) { - require.NotNil(t, PostProcessFile("/non/existing/path")) + err := PostProcessFile("/non/existing/path") + VerifyErrorClarification(t, constants.Internal_FailedToOpenFileForReading, err) } func Test_postProcessFile(t *testing.T) { @@ -227,3 +233,76 @@ func Test_saveScriptFile(t *testing.T) { require.Nil(t, err) require.Equal(t, content, string(result)) } + +func TestGetDownloaders_NonBlobURL_ReturnsPublicOnly(t *testing.T) { + publicURL := "https://example.com/scripts/a.sh" + + mock := &mockMsiDownloader{providerToReturn: providerSuccess()} + downloaders, err := getDownloaders(publicURL, nil, mock) + + require.Nil(t, err) + require.Len(t, downloaders, 1, "non-blob URL must return only public downloader") + require.Equal(t, 0, mock.calledGet+mock.calledByClientID+mock.calledByObjectID, + "msi downloader must not be used for non-blob URL") +} + +func TestGetDownloaders_EmptyURL_ReturnsClarification(t *testing.T) { + dl, err := getDownloaders("", nil, &mockMsiDownloader{providerToReturn: providerSuccess()}) + require.Nil(t, dl) + VerifyErrorClarification(t, constants.FileDownload_Empty, err) +} + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} + +func TestGetDownloaders_BlobURL_BothClientAndObjectID_ReturnsClarification(t *testing.T) { + blobURL := "https://acct.blob.core.windows.net/container/blob.txt" + + mi := &handlersettings.RunCommandManagedIdentity{ + ClientId: "11111111-1111-1111-1111-111111111111", + ObjectId: "22222222-2222-2222-2222-222222222222", + } + + mock := &mockMsiDownloader{providerToReturn: providerSuccess()} + downloaders, err := getDownloaders(blobURL, mi, mock) + + require.Nil(t, downloaders) + VerifyErrorClarification(t, constants.CustomerInput_ClientIdObjectIdBothSpecified, err) +} + +// MsiProvider is invoked as: _, err := msiProvider() +type mockMsiDownloader struct { + calledGet int + calledByClientID int + calledByObjectID int + lastURL string + lastClientID string + lastObjectID string + providerToReturn download.MsiProvider +} + +func (m *mockMsiDownloader) GetMsiProvider(url string) download.MsiProvider { + m.calledGet++ + m.lastURL = url + return m.providerToReturn +} + +func (m *mockMsiDownloader) GetMsiProviderByClientId(url, clientId string) download.MsiProvider { + m.calledByClientID++ + m.lastURL = url + m.lastClientID = clientId + return m.providerToReturn +} + +func (m *mockMsiDownloader) GetMsiProviderByObjectId(url, objectId string) download.MsiProvider { + m.calledByObjectID++ + m.lastURL = url + m.lastObjectID = objectId + return m.providerToReturn +} + +func providerSuccess() download.MsiProvider { + return func() (msi.Msi, error) { return msi.Msi{}, nil } +} diff --git a/internal/goalstate/goalstate.go b/internal/goalstate/goalstate.go index 8d592bd..8799107 100644 --- a/internal/goalstate/goalstate.go +++ b/internal/goalstate/goalstate.go @@ -4,6 +4,7 @@ import ( "fmt" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/cleanup" commands "github.com/Azure/run-command-handler-linux/internal/cmds" "github.com/Azure/run-command-handler-linux/internal/commandProcessor" @@ -33,20 +34,20 @@ var statusToCommandMap = map[string]string{ // setting: The settings for the command. // notifier: The notifier to send the status to the HGAP. This is a notifier that must have been initialized and a status observer must have been added to it. // Returns the exit code and an error if there was an issue executing the goal state. -func HandleImmediateGoalState(ctx *log.Context, setting settings.SettingsCommon, notifier *observer.Notifier) (int, error) { +func HandleImmediateGoalState(ctx *log.Context, setting settings.SettingsCommon, notifier *observer.Notifier) (int, *vmextension.ErrorWithClarification) { done := make(chan bool) err := make(chan error) go startAsync(ctx, setting, notifier, done, err) select { case e := <-err: ctx.Log("error", fmt.Sprintf("error when trying to execute goal state: %v", e)) - return constants.ExitCode_ImmediateTaskFailed, errors.Wrapf(e, "error when trying to execute goal state") + return constants.ImmediateRC_UnknownFailure, vmextension.NewErrorWithClarificationPtr(constants.ImmediateRC_UnknownFailure, errors.New("error when trying to execute goal state")) case <-done: ctx.Log("message", "goal state successfully finished") return constants.ExitCode_Okay, nil case <-time.After(time.Minute * time.Duration(maxExecutionTimeInMinutes)): ctx.Log("message", "timeout when trying to execute goal state") - return constants.ExitCode_ImmediateTaskTimeout, errors.New("timeout when trying to execute goal state") + return constants.ImmediateRC_TaskTimeout, vmextension.NewErrorWithClarificationPtr(constants.ImmediateRC_TaskTimeout, errors.New("timeout when trying to execute goal state")) } } @@ -109,7 +110,7 @@ func startAsync(ctx *log.Context, setting settings.SettingsCommon, notifier *obs } // Overwrite function to report status to HGAP. This function prepares the status to be sent to the HGAP and then calls the notifier to send it. - cmd.Functions.ReportStatus = func(ctx *log.Context, _ types.HandlerEnvironment, metadata types.RCMetadata, statusType types.StatusType, c types.Cmd, msg string) error { + cmd.Functions.ReportStatus = func(ctx *log.Context, _ types.HandlerEnvironment, metadata types.RCMetadata, statusType types.StatusType, c types.Cmd, msg string, exitcode ...int) error { if !c.ShouldReportStatus { ctx.Log("status", fmt.Sprintf("status not reported for operation %v (by design)", c.Name)) return nil diff --git a/internal/goalstate/goalstate_test.go b/internal/goalstate/goalstate_test.go index 014b56e..1a8194a 100644 --- a/internal/goalstate/goalstate_test.go +++ b/internal/goalstate/goalstate_test.go @@ -47,7 +47,7 @@ func Test_handleSkippedImmediateGoalState_NotifyObserver(t *testing.T) { instView := types.RunCommandInstanceView{ ExecutionState: types.Failed, ExecutionMessage: "Execution failed", - ExitCode: constants.ExitCode_SkippedImmediateGoalState, + ExitCode: constants.ImmediateRC_CommandSkipped, Output: "", Error: errorMsg, StartTime: time.Now().UTC().Format(time.RFC3339), diff --git a/internal/goalstate/goalstatefromvmsettings.go b/internal/goalstate/goalstatefromvmsettings.go index 8c30147..cd243a6 100644 --- a/internal/goalstate/goalstatefromvmsettings.go +++ b/internal/goalstate/goalstatefromvmsettings.go @@ -4,20 +4,21 @@ import ( "fmt" "strings" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/hostgacommunicator" "github.com/go-kit/kit/log" "github.com/pkg/errors" ) -func GetImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgacommunicator.IHostGACommunicator, lastProcessedETag string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, error) { +func GetImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgacommunicator.IHostGACommunicator, lastProcessedETag string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, *vmextension.ErrorWithClarification) { if communicator == nil { - return nil, lastProcessedETag, errors.New("communicator cannot be nil") + return nil, lastProcessedETag, vmextension.NewErrorWithClarificationPtr(constants.Hgap_InternalArgumentError, errors.New("communicator cannot be nil")) } responseData, err := communicator.GetImmediateVMSettings(ctx, lastProcessedETag) if err != nil { - return nil, lastProcessedETag, errors.Wrapf(err, "failed to retrieve immediate VMSettings") + return nil, lastProcessedETag, err } if responseData != nil && responseData.Modified { diff --git a/internal/goalstate/goalstatefromvmsettings_test.go b/internal/goalstate/goalstatefromvmsettings_test.go index bc0b956..cc48ace 100644 --- a/internal/goalstate/goalstatefromvmsettings_test.go +++ b/internal/goalstate/goalstatefromvmsettings_test.go @@ -5,6 +5,8 @@ import ( "os" "testing" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/goalstate" "github.com/Azure/run-command-handler-linux/internal/hostgacommunicator" "github.com/Azure/run-command-handler-linux/internal/settings" @@ -14,7 +16,7 @@ import ( type TestCommunicator struct{} -func (t *TestCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, error) { +func (t *TestCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, *vmextension.ErrorWithClarification) { extName, seqNum := "testExtension", 5 immediateGoalState := hostgacommunicator.ImmediateExtensionGoalState{ Name: "Microsoft.CPlat.Core.RunCommandHandlerLinux", @@ -82,19 +84,19 @@ func (t *TestCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) type BadCommunicator struct{} -func (t *BadCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, error) { - return nil, errors.New("http expected failure") +func (t *BadCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, *vmextension.ErrorWithClarification) { + return nil, vmextension.NewErrorWithClarificationPtr(42, errors.New("http expected failure")) } type NilCommunicator struct{} -func (t *NilCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, error) { +func (t *NilCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, *vmextension.ErrorWithClarification) { return nil, nil } type EmptyCommunicator struct{} -func (t *EmptyCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, error) { +func (t *EmptyCommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*hostgacommunicator.ResponseData, *vmextension.ErrorWithClarification) { return &hostgacommunicator.ResponseData{VMSettings: &hostgacommunicator.VMImmediateExtensionsGoalState{}, ETag: "123456", Modified: true}, nil } @@ -110,10 +112,16 @@ func Test_GetFilteredImmediateVMSettingsFailedToRetrieve(t *testing.T) { ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) badCommunicator := new(BadCommunicator) _, _, err := goalstate.GetImmediateRunCommandGoalStates(ctx, badCommunicator, "") - require.ErrorContains(t, err, "failed to retrieve immediate VMSettings") require.ErrorContains(t, err, "http expected failure") } +func Test_GetFilteredImmediateVMSettings_NoCommunicator(t *testing.T) { + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + _, _, ewc := goalstate.GetImmediateRunCommandGoalStates(ctx, nil, "") + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, constants.Hgap_InternalArgumentError, ewc.ErrorCode, "Expected error %d but received %d", constants.Hgap_InternalArgumentError, ewc.ErrorCode) +} + func Test_GetFilteredImmediateVMSettingsHandleEmptyResults(t *testing.T) { ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) nilCommunicator := new(NilCommunicator) diff --git a/internal/handlersettings/handlerenv.go b/internal/handlersettings/handlerenv.go index e6f6489..1b5f17f 100644 --- a/internal/handlersettings/handlerenv.go +++ b/internal/handlersettings/handlerenv.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/types" ) @@ -17,10 +19,10 @@ const HandlerEnvFileName = "HandlerEnvironment.json" // GetHandlerEnv locates the HandlerEnvironment.json file by assuming it lives // next to or one level above the extension handler (read: this) executable, // reads, parses and returns it. -func GetHandlerEnv() (he types.HandlerEnvironment, _ error) { +func GetHandlerEnv() (he types.HandlerEnvironment, _ *vmextension.ErrorWithClarification) { dir, err := scriptDir() if err != nil { - return he, fmt.Errorf("vmextension: cannot find base directory of the running process: %v", err) + return he, vmextension.NewErrorWithClarificationPtr(constants.HandlerEnv_CouldNotFindBaseDirectory, fmt.Errorf("vmextension: cannot find base directory of the running process: %v", err)) } paths := []string{ filepath.Join(dir, HandlerEnvFileName), // this level (i.e. executable is in [EXT_NAME]/.) @@ -30,14 +32,14 @@ func GetHandlerEnv() (he types.HandlerEnvironment, _ error) { for _, p := range paths { o, err := os.ReadFile(p) if err != nil && !os.IsNotExist(err) { - return he, fmt.Errorf("vmextension: error examining HandlerEnvironment at '%s': %v", p, err) + return he, vmextension.NewErrorWithClarificationPtr(constants.HandlerEnv_HandlingError, fmt.Errorf("vmextension: error examining HandlerEnvironment at '%s': %v", p, err)) } else if err == nil { b = o break } } if b == nil { - return he, fmt.Errorf("vmextension: Cannot find HandlerEnvironment at paths: %s", strings.Join(paths, ", ")) + return he, vmextension.NewErrorWithClarificationPtr(constants.HandlerEnv_NotFound, fmt.Errorf("vmextension: Cannot find HandlerEnvironment at paths: %s", strings.Join(paths, ", "))) } return ParseHandlerEnv(b) } @@ -53,14 +55,14 @@ func scriptDir() (string, error) { // ParseHandlerEnv parses the // /var/lib/waagent/[extension]/HandlerEnvironment.json format. -func ParseHandlerEnv(b []byte) (he types.HandlerEnvironment, _ error) { +func ParseHandlerEnv(b []byte) (he types.HandlerEnvironment, _ *vmextension.ErrorWithClarification) { var hf []types.HandlerEnvironment if err := json.Unmarshal(b, &hf); err != nil { - return he, fmt.Errorf("vmextension: failed to parse handler env: %v", err) + return he, vmextension.NewErrorWithClarificationPtr(constants.HandlerEnv_UnmarshalFailed, fmt.Errorf("vmextension: failed to parse handler env: %v", err)) } if len(hf) != 1 { - return he, fmt.Errorf("vmextension: expected 1 config in parsed HandlerEnvironment, found: %v", len(hf)) + return he, vmextension.NewErrorWithClarificationPtr(constants.HandlerEnv_InvalidConfigCount, fmt.Errorf("vmextension: expected 1 config in parsed HandlerEnvironment, found: %v", len(hf))) } return hf[0], nil } diff --git a/internal/handlersettings/handlerenv_test.go b/internal/handlersettings/handlerenv_test.go new file mode 100644 index 0000000..2699ce5 --- /dev/null +++ b/internal/handlersettings/handlerenv_test.go @@ -0,0 +1,161 @@ +package handlersettings + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/types" + "github.com/stretchr/testify/require" +) + +func writeHandlerEnvJSON(t *testing.T, path string, he types.HandlerEnvironment) { + t.Helper() + b, err := json.Marshal([]types.HandlerEnvironment{he}) + require.NoError(t, err) + require.NoError(t, os.WriteFile(path, b, 0o644)) +} + +func TestParseHandlerEnv_UnmarshalFailed(t *testing.T) { + _, err := ParseHandlerEnv([]byte("{not-json")) + VerifyErrorClarification(t, constants.HandlerEnv_UnmarshalFailed, err) +} + +func TestParseHandlerEnv_InvalidConfigCount_Zero(t *testing.T) { + b, err := json.Marshal([]types.HandlerEnvironment{}) + require.NoError(t, err) + + _, ewc := ParseHandlerEnv(b) + VerifyErrorClarification(t, constants.HandlerEnv_InvalidConfigCount, ewc) +} + +func TestParseHandlerEnv_InvalidConfigCount_Two(t *testing.T) { + b, err := json.Marshal([]types.HandlerEnvironment{ + {Version: 1.0}, + {Version: 1.0}, + }) + require.NoError(t, err) + + _, ewc := ParseHandlerEnv(b) + VerifyErrorClarification(t, constants.HandlerEnv_InvalidConfigCount, ewc) +} + +func TestParseHandlerEnv_Success(t *testing.T) { + want := types.HandlerEnvironment{ + Version: 1.0, + HandlerEnvironment: types.HandlerEnvironmentDetails{ + LogFolder: "/var/log/azure/ext/log", + ConfigFolder: "/var/lib/waagent/ext/config", + StatusFolder: "/var/lib/waagent/ext/status", + HeartbeatFile: "/var/lib/waagent/ext/heartbeat", + DeploymentID: "dep", + RoleName: "role", + Instance: "inst", + HostResolverAddress: "168.63.129.16", + EventsFolder: "/var/log/azure/ext/events", + }, + } + + b, err := json.Marshal([]types.HandlerEnvironment{want}) + require.NoError(t, err) + + got, ewc := ParseHandlerEnv(b) + require.Nil(t, ewc) + require.Equal(t, want, got) +} + +func TestGetHandlerEnv_FindsHandlerEnvironmentNextToExecutable(t *testing.T) { + tmp := t.TempDir() + + // Simulate "executable" location: tmp/ext/ + exeDir := filepath.Join(tmp, "ext") + require.NoError(t, os.MkdirAll(exeDir, 0o755)) + + exePath := filepath.Join(exeDir, "run-command-handler-linux") + _ = os.WriteFile(exePath, []byte("dummy"), 0o755) + + // HandlerEnvironment.json placed next to executable + want := types.HandlerEnvironment{Version: 1.0} + writeHandlerEnvJSON(t, filepath.Join(exeDir, HandlerEnvFileName), want) + + origArgs0 := os.Args[0] + t.Cleanup(func() { os.Args[0] = origArgs0 }) + os.Args[0] = exePath + + got, ewc := GetHandlerEnv() + require.Nil(t, ewc) + require.Equal(t, want, got) +} + +func TestGetHandlerEnv_FindsHandlerEnvironmentOneLevelAboveExecutable(t *testing.T) { + tmp := t.TempDir() + + // Simulate "bin" layout: tmp/ext/bin/, and HandlerEnvironment.json in tmp/ext/ + extDir := filepath.Join(tmp, "ext") + binDir := filepath.Join(extDir, "bin") + require.NoError(t, os.MkdirAll(binDir, 0o755)) + + exePath := filepath.Join(binDir, "run-command-handler-linux") + _ = os.WriteFile(exePath, []byte("dummy"), 0o755) + + want := types.HandlerEnvironment{Version: 1.0} + writeHandlerEnvJSON(t, filepath.Join(extDir, HandlerEnvFileName), want) + + origArgs0 := os.Args[0] + t.Cleanup(func() { os.Args[0] = origArgs0 }) + os.Args[0] = exePath + + got, ewc := GetHandlerEnv() + require.Nil(t, ewc) + require.Equal(t, want, got) +} + +func TestGetHandlerEnv_NotFound(t *testing.T) { + tmp := t.TempDir() + + exeDir := filepath.Join(tmp, "ext") + require.NoError(t, os.MkdirAll(exeDir, 0o755)) + + exePath := filepath.Join(exeDir, "run-command-handler-linux") + _ = os.WriteFile(exePath, []byte("dummy"), 0o755) + + origArgs0 := os.Args[0] + t.Cleanup(func() { os.Args[0] = origArgs0 }) + os.Args[0] = exePath + + _, err := GetHandlerEnv() + VerifyErrorClarification(t, constants.HandlerEnv_NotFound, err) +} + +func TestGetHandlerEnv_HandlingError_OnReadFailure(t *testing.T) { + // On Windows, chmod perms tests are unreliable due to ACL behavior; skip. + if runtime.GOOS == "windows" { + t.Skip("permission-based read failure test is unreliable on Windows") + } + + tmp := t.TempDir() + + exeDir := filepath.Join(tmp, "ext") + require.NoError(t, os.MkdirAll(exeDir, 0o755)) + + exePath := filepath.Join(exeDir, "run-command-handler-linux") + _ = os.WriteFile(exePath, []byte("dummy"), 0o755) + + // Create a HandlerEnvironment.json but remove read permissions to trigger an error != IsNotExist + envPath := filepath.Join(exeDir, HandlerEnvFileName) + writeHandlerEnvJSON(t, envPath, types.HandlerEnvironment{Version: 1.0}) + require.NoError(t, os.Chmod(envPath, 0o000)) + + origArgs0 := os.Args[0] + t.Cleanup(func() { + os.Args[0] = origArgs0 + _ = os.Chmod(envPath, 0o644) // restore for cleanup on some fs + }) + os.Args[0] = exePath + + _, err := GetHandlerEnv() + VerifyErrorClarification(t, constants.HandlerEnv_HandlingError, err) +} diff --git a/internal/handlersettings/handlersettings.go b/internal/handlersettings/handlersettings.go index dfecd8f..b047b54 100644 --- a/internal/handlersettings/handlersettings.go +++ b/internal/handlersettings/handlersettings.go @@ -1,6 +1,7 @@ package handlersettings import ( + "github.com/Azure/azure-extension-platform/vmextension" "github.com/go-kit/kit/log" "github.com/pkg/errors" ) @@ -11,7 +12,7 @@ var ( // parseAndValidateSettings reads configuration from configFolder, decrypts it, // runs JSON-schema and logical validation on it and returns it back. -func ParseAndValidateSettings(ctx *log.Context, configFilePath string) (h HandlerSettings, _ error) { +func ParseAndValidateSettings(ctx *log.Context, configFilePath string) (h HandlerSettings, _ *vmextension.ErrorWithClarification) { ctx.Log("event", "reading configuration from "+configFilePath) pubJSON, protJSON, err := readSettings(configFilePath) if err != nil { @@ -21,13 +22,13 @@ func ParseAndValidateSettings(ctx *log.Context, configFilePath string) (h Handle ctx.Log("event", "parsing configuration json") if err := UnmarshalHandlerSettings(pubJSON, protJSON, &h.PublicSettings, &h.ProtectedSettings); err != nil { - return h, errors.Wrap(err, "json parsing error") + return h, err } ctx.Log("event", "parsed configuration json") ctx.Log("event", "validating configuration logically") if err := h.validate(); err != nil { - return h, errors.Wrap(err, "invalid configuration") + return h, err } ctx.Log("event", "validated configuration") return h, nil @@ -36,8 +37,7 @@ func ParseAndValidateSettings(ctx *log.Context, configFilePath string) (h Handle // readSettings uses specified configFilePath (comes from HandlerEnvironment) to // decrypt and parse the public/protected settings of the extension handler into // JSON objects. -func readSettings(configFilePath string) (pubSettingsJSON, protSettingsJSON map[string]interface{}, err error) { +func readSettings(configFilePath string) (pubSettingsJSON, protSettingsJSON map[string]interface{}, err *vmextension.ErrorWithClarification) { pubSettingsJSON, protSettingsJSON, err = ReadSettings(configFilePath) - err = errors.Wrapf(err, "error reading extension configuration") return } diff --git a/internal/handlersettings/handlersettings_test.go b/internal/handlersettings/handlersettings_test.go index 303aceb..7eb9666 100644 --- a/internal/handlersettings/handlersettings_test.go +++ b/internal/handlersettings/handlersettings_test.go @@ -12,7 +12,7 @@ func Test_handlerSettingsValidate(t *testing.T) { require.Equal(t, errSourceNotSpecified, HandlerSettings{ PublicSettings{Source: &ScriptSource{Script: "foo", ScriptURI: "bar"}}, ProtectedSettings{}, - }.validate()) + }.validate().Err) // // commandToExecute not specified // require.Equal(t, errCmdMissing, handlerSettings{ diff --git a/internal/handlersettings/handlersettingscommon.go b/internal/handlersettings/handlersettingscommon.go index 50f6e51..d9eb1b0 100644 --- a/internal/handlersettings/handlersettingscommon.go +++ b/internal/handlersettings/handlersettingscommon.go @@ -9,6 +9,8 @@ import ( "os/exec" "path/filepath" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/settings" "github.com/pkg/errors" ) @@ -24,20 +26,20 @@ type RunTimeSettingsFile struct { // ReadSettings locates the .settings file and returns public settings // JSON, and protected settings JSON (by decrypting it with the keys in // configFolder). -func ReadSettings(configFilePath string) (public, protected map[string]interface{}, _ error) { +func ReadSettings(configFilePath string) (public, protected map[string]interface{}, _ *vmextension.ErrorWithClarification) { // cf, err := settingsPath(configFolder) // if err != nil { // return nil, nil, fmt.Errorf("canot locate settings file: %v", err) // } hs, err := parseHandlerSettingsFile(configFilePath) if err != nil { - return nil, nil, fmt.Errorf("error parsing settings file: %v", err) + return nil, nil, err } public = hs.PublicSettings configFolder := filepath.Dir(configFilePath) if err := unmarshalProtectedSettings(configFolder, hs, &protected); err != nil { - return nil, nil, fmt.Errorf("failed to parse protected settings: %v", err) + return nil, nil, err } return public, protected, nil } @@ -45,35 +47,35 @@ func ReadSettings(configFilePath string) (public, protected map[string]interface // UnmarshalHandlerSettings unmarshals given publicSettings/protectedSettings types // assumed underlying values are JSON into references publicV/protectedV respectively // (of struct types that contain structured fields for settings). -func UnmarshalHandlerSettings(publicSettings, protectedSettings map[string]interface{}, publicV, protectedV interface{}) error { +func UnmarshalHandlerSettings(publicSettings, protectedSettings map[string]interface{}, publicV, protectedV interface{}) *vmextension.ErrorWithClarification { if err := unmarshalSettings(publicSettings, &publicV); err != nil { - return fmt.Errorf("failed to unmarshal public settings: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_UnmarshalPublicSettingsFailed, fmt.Errorf("failed to unmarshal public settings: %v", err)) } if err := unmarshalSettings(protectedSettings, &protectedV); err != nil { - return fmt.Errorf("failed to unmarshal protected settings: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_UnmarshalProtectedSettingsFailed, fmt.Errorf("failed to unmarshal protected settings: %v", err)) } return nil } // unmarshalSettings makes a round-trip JSON marshaling and unmarshaling // from in (assumed map[interface]{}) to v (actual settings type). -func unmarshalSettings(in interface{}, v interface{}) error { +func unmarshalSettings(in interface{}, v interface{}) *vmextension.ErrorWithClarification { s, err := json.Marshal(in) if err != nil { - return fmt.Errorf("failed to marshal into json: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_UnmarshalSettingsFailed, fmt.Errorf("failed to marshal into json: %v", err)) } if err := json.Unmarshal(s, &v); err != nil { - return fmt.Errorf("failed to unmarshal json: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_UnmarshalSettingsFailed, fmt.Errorf("failed to unmarshal json: %v", err)) } return nil } // parseHandlerSettings parses a handler settings file (e.g. 0.settings) and // returns it as a structured object. -func parseHandlerSettingsFile(path string) (h settings.SettingsCommon, _ error) { +func parseHandlerSettingsFile(path string) (h settings.SettingsCommon, _ *vmextension.ErrorWithClarification) { b, err := os.ReadFile(path) if err != nil { - return h, fmt.Errorf("error reading %s: %v", path, err) + return h, vmextension.NewErrorWithClarificationPtr(constants.Internal_CouldNotParseSettings, fmt.Errorf("error reading %s: %v", path, err)) } if len(b) == 0 { // if no config is specified, we get an empty file return h, nil @@ -81,10 +83,10 @@ func parseHandlerSettingsFile(path string) (h settings.SettingsCommon, _ error) var f HandlerSettingsFile if err := json.Unmarshal(b, &f); err != nil { - return h, fmt.Errorf("error parsing json: %v", err) + return h, vmextension.NewErrorWithClarificationPtr(constants.Internal_InvalidHandlerSettingsJson, fmt.Errorf("error parsing json: %v", err)) } if len(f.RuntimeSettings) != 1 { - return h, fmt.Errorf("wrong runtimeSettings count. expected:1, got:%d", len(f.RuntimeSettings)) + return h, vmextension.NewErrorWithClarificationPtr(constants.Internal_InvalidHandlerSettingsCount, fmt.Errorf("wrong runtimeSettings count. expected:1, got:%d", len(f.RuntimeSettings))) } return f.RuntimeSettings[0].HandlerSettings, nil } @@ -92,17 +94,17 @@ func parseHandlerSettingsFile(path string) (h settings.SettingsCommon, _ error) // unmarshalProtectedSettings decodes the protected settings from handler // runtime settings JSON file, decrypts it using the certificates and unmarshals // into the given struct v. -func unmarshalProtectedSettings(configFolder string, hs settings.SettingsCommon, v interface{}) error { +func unmarshalProtectedSettings(configFolder string, hs settings.SettingsCommon, v interface{}) *vmextension.ErrorWithClarification { if hs.ProtectedSettingsBase64 == "" { return nil } if hs.SettingsCertThumbprint == "" { - return errors.New("HandlerSettings has protected settings but no cert thumbprint") + return vmextension.NewErrorWithClarificationPtr(constants.Internal_NoHandlerSettingsThumbprint, errors.New("HandlerSettings has protected settings but no cert thumbprint")) } decoded, err := base64.StdEncoding.DecodeString(hs.ProtectedSettingsBase64) if err != nil { - return fmt.Errorf("failed to decode base64: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_HandlerSettingsFailedToDecode, fmt.Errorf("failed to decode base64: %v", err)) } // go two levels up where certs are placed (/var/lib/waagent) @@ -131,13 +133,13 @@ func unmarshalProtectedSettings(configFolder string, hs settings.SettingsCommon, cmd.Stdout = &bOut cmd.Stderr = &bErr if err := cmd.Run(); err != nil { - return errors.Wrapf(errMsg, "decrypting protected settings with smime command failed: error=%v stderr=%s", err, bErr.String()) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_DecryptingProtectedSettingsFailed, errors.Wrapf(errMsg, "decrypting protected settings with smime command failed: error=%v stderr=%s", err, bErr.String())) } } // decrypted: json object for protected settings if err := json.Unmarshal(bOut.Bytes(), &v); err != nil { - return fmt.Errorf("failed to unmarshal decrypted settings json: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Internal_UnmarshalProtectedSettingsFailed, fmt.Errorf("failed to unmarshal decrypted settings json: %v", err)) } return nil } diff --git a/internal/handlersettings/handlersettingscommon_test.go b/internal/handlersettings/handlersettingscommon_test.go new file mode 100644 index 0000000..03e4ad6 --- /dev/null +++ b/internal/handlersettings/handlersettingscommon_test.go @@ -0,0 +1,325 @@ +package handlersettings + +import ( + "encoding/base64" + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/settings" + "github.com/stretchr/testify/require" +) + +func writeFile(t *testing.T, path string, content []byte, mode os.FileMode) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, content, mode)) +} + +func makeSettingsFile(t *testing.T, path string, hs settings.SettingsCommon) { + t.Helper() + + f := HandlerSettingsFile{ + RuntimeSettings: []RunTimeSettingsFile{ + {HandlerSettings: hs}, + }, + } + b, err := json.Marshal(f) + require.NoError(t, err) + writeFile(t, path, b, 0o644) +} + +// Creates a temp PATH entry containing a fake "openssl" script. +// behavior: +// - if cmsShouldFail=true, cms exits nonzero and writes stderr +// - if smimeShouldFail=true, smime exits nonzero and writes stderr +// - success prints outputJSON to stdout +func installFakeOpenSSL(t *testing.T, cmsShouldFail, smimeShouldFail bool, outputJSON string) (restore func()) { + t.Helper() + + if runtime.GOOS == "windows" { + t.Skip("These tests rely on creating a fake openssl script in PATH; adapt for Windows if needed.") + } + + tmp := t.TempDir() + fake := filepath.Join(tmp, "openssl") + + script := "#!/bin/sh\n" + + "cmd=\"$1\"\n" + + "shift\n" + + "if [ \"$cmd\" = \"cms\" ]; then\n" + + " if [ \"" + boolToStr(cmsShouldFail) + "\" = \"true\" ]; then\n" + + " echo \"cms failed\" 1>&2\n" + + " exit 1\n" + + " fi\n" + + " echo '" + escapeSingleQuotes(outputJSON) + "'\n" + + " exit 0\n" + + "fi\n" + + "if [ \"$cmd\" = \"smime\" ]; then\n" + + " if [ \"" + boolToStr(smimeShouldFail) + "\" = \"true\" ]; then\n" + + " echo \"smime failed\" 1>&2\n" + + " exit 1\n" + + " fi\n" + + " echo '" + escapeSingleQuotes(outputJSON) + "'\n" + + " exit 0\n" + + "fi\n" + + "echo \"unexpected openssl subcommand: $cmd\" 1>&2\n" + + "exit 2\n" + + writeFile(t, fake, []byte(script), 0o755) + + origPath := os.Getenv("PATH") + require.NoError(t, os.Setenv("PATH", tmp+string(os.PathListSeparator)+origPath)) + + return func() { + _ = os.Setenv("PATH", origPath) + } +} + +func boolToStr(b bool) string { + if b { + return "true" + } + return "false" +} + +func escapeSingleQuotes(s string) string { + // for shell: wrap in single quotes, escape internal single quotes + // ' -> '"'"' + out := "" + for _, r := range s { + if r == '\'' { + out += "'\"'\"'" + } else { + out += string(r) + } + } + return out +} + +func makeConfigAndCerts(t *testing.T, thumb string) (configFolder string) { + t.Helper() + + root := t.TempDir() + // config folder is .../some/ext/config ; certs are two levels up from configFolder + // so: configFolder = root/ext/config, certs live in root/{thumb}.crt and root/{thumb}.prv + configFolder = filepath.Join(root, "ext", "config") + require.NoError(t, os.MkdirAll(configFolder, 0o755)) + + crt := filepath.Join(configFolder, "..", "..", thumb+".crt") + prv := filepath.Join(configFolder, "..", "..", thumb+".prv") + writeFile(t, crt, []byte("dummycrt"), 0o644) + writeFile(t, prv, []byte("dummyprv"), 0o600) + + return configFolder +} + +/* -------------------- parseHandlerSettingsFile -------------------- */ + +func TestParseHandlerSettingsFile_ReadError(t *testing.T) { + _, err := parseHandlerSettingsFile(filepath.Join(t.TempDir(), "missing.settings")) + VerifyErrorClarification(t, constants.Internal_CouldNotParseSettings, err) +} + +func TestParseHandlerSettingsFile_EmptyFile_OK(t *testing.T) { + p := filepath.Join(t.TempDir(), "0.settings") + writeFile(t, p, []byte{}, 0o644) + + got, err := parseHandlerSettingsFile(p) + require.Nil(t, err) + // empty settings file -> zero-value SettingsCommon + require.Equal(t, settings.SettingsCommon{}, got) +} + +func TestParseHandlerSettingsFile_InvalidJSON(t *testing.T) { + p := filepath.Join(t.TempDir(), "0.settings") + writeFile(t, p, []byte("{not-json"), 0o644) + + _, err := parseHandlerSettingsFile(p) + VerifyErrorClarification(t, constants.Internal_InvalidHandlerSettingsJson, err) +} + +func TestParseHandlerSettingsFile_WrongRuntimeSettingsCount(t *testing.T) { + p := filepath.Join(t.TempDir(), "0.settings") + + f := HandlerSettingsFile{ + RuntimeSettings: []RunTimeSettingsFile{ + {}, {}, + }, + } + b, err := json.Marshal(f) + require.NoError(t, err) + writeFile(t, p, b, 0o644) + + _, ewc := parseHandlerSettingsFile(p) + VerifyErrorClarification(t, constants.Internal_InvalidHandlerSettingsCount, ewc) +} + +func TestParseHandlerSettingsFile_Success(t *testing.T) { + p := filepath.Join(t.TempDir(), "0.settings") + + hs := settings.SettingsCommon{ + PublicSettings: map[string]interface{}{"k": "v"}, + } + makeSettingsFile(t, p, hs) + + got, err := parseHandlerSettingsFile(p) + require.Nil(t, err) + require.Equal(t, hs.PublicSettings, got.PublicSettings) +} + +/* -------------------- ReadSettings -------------------- */ + +func TestReadSettings_NoProtected_ReturnsPublicAndNilProtected(t *testing.T) { + p := filepath.Join(t.TempDir(), "0.settings") + hs := settings.SettingsCommon{ + PublicSettings: map[string]interface{}{"hello": "world"}, + ProtectedSettingsBase64: "", + SettingsCertThumbprint: "", + } + makeSettingsFile(t, p, hs) + + pub, prot, err := ReadSettings(p) + require.Nil(t, err) + require.Equal(t, hs.PublicSettings, pub) + require.Nil(t, prot) // nothing set +} + +func TestReadSettings_PropagatesParseError(t *testing.T) { + _, _, ewc := ReadSettings(filepath.Join(t.TempDir(), "missing.settings")) + VerifyErrorClarification(t, constants.Internal_CouldNotParseSettings, ewc) +} + +/* -------------------- unmarshalSettings + UnmarshalHandlerSettings -------------------- */ + +func TestUnmarshalSettings_MarshalError_ReturnsClarification(t *testing.T) { + // json.Marshal fails on channel / func values + in := map[string]interface{}{"bad": make(chan int)} + var out map[string]interface{} + err := unmarshalSettings(in, &out) + VerifyErrorClarification(t, constants.Internal_UnmarshalSettingsFailed, err) +} + +func TestUnmarshalHandlerSettings_PublicUnmarshalFails(t *testing.T) { + public := map[string]interface{}{"bad": make(chan int)} // will fail marshal + protected := map[string]interface{}{"ok": "x"} + var pubV map[string]interface{} + var protV map[string]interface{} + + err := UnmarshalHandlerSettings(public, protected, &pubV, &protV) + VerifyErrorClarification(t, constants.Internal_UnmarshalPublicSettingsFailed, err) +} + +func TestUnmarshalHandlerSettings_ProtectedUnmarshalFails(t *testing.T) { + public := map[string]interface{}{"ok": "x"} + protected := map[string]interface{}{"bad": make(chan int)} // will fail marshal + var pubV map[string]interface{} + var protV map[string]interface{} + + err := UnmarshalHandlerSettings(public, protected, &pubV, &protV) + VerifyErrorClarification(t, constants.Internal_UnmarshalProtectedSettingsFailed, err) +} + +func TestUnmarshalHandlerSettings_Success_PopulatesStructs(t *testing.T) { + type Pub struct { + A string `json:"a"` + } + type Prot struct { + B int `json:"b"` + } + + public := map[string]interface{}{"a": "hello"} + protected := map[string]interface{}{"b": 42} + var pub Pub + var prot Prot + + err := UnmarshalHandlerSettings(public, protected, &pub, &prot) + require.Nil(t, err) + + require.Equal(t, "hello", pub.A) + require.Equal(t, 42, prot.B) +} + +/* -------------------- unmarshalProtectedSettings -------------------- */ + +func TestUnmarshalProtectedSettings_NoProtectedSettings_ReturnsNil(t *testing.T) { + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: "", + SettingsCertThumbprint: "thumb", + } + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + require.Nil(t, err) +} + +func TestUnmarshalProtectedSettings_ProtectedButNoThumbprint(t *testing.T) { + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: base64.StdEncoding.EncodeToString([]byte("anything")), + SettingsCertThumbprint: "", + } + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + VerifyErrorClarification(t, constants.Internal_NoHandlerSettingsThumbprint, err) +} + +func TestUnmarshalProtectedSettings_Base64DecodeFails(t *testing.T) { + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: "!!!notbase64!!!", + SettingsCertThumbprint: "thumb", + } + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + VerifyErrorClarification(t, constants.Internal_HandlerSettingsFailedToDecode, err) +} + +func TestUnmarshalProtectedSettings_CmsFails_SmimeSucceeds(t *testing.T) { + restore := installFakeOpenSSL(t, true, false, `{"p":"ok"}`) + defer restore() + + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: base64.StdEncoding.EncodeToString([]byte("ciphertext")), + SettingsCertThumbprint: "thumb", + } + + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + require.Nil(t, err) + require.Equal(t, "ok", out["p"]) +} + +func TestUnmarshalProtectedSettings_CmsFails_SmimeFails_ReturnsDecryptError(t *testing.T) { + restore := installFakeOpenSSL(t, true, true, `{"p":"ok"}`) + defer restore() + + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: base64.StdEncoding.EncodeToString([]byte("ciphertext")), + SettingsCertThumbprint: "thumb", + } + + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + VerifyErrorClarification(t, constants.Internal_DecryptingProtectedSettingsFailed, err) +} + +func TestUnmarshalProtectedSettings_DecryptedNotJSON_ReturnsUnmarshalProtectedFailed(t *testing.T) { + restore := installFakeOpenSSL(t, false, false, `not-json`) + defer restore() + + cfg := makeConfigAndCerts(t, "thumb") + hs := settings.SettingsCommon{ + ProtectedSettingsBase64: base64.StdEncoding.EncodeToString([]byte("ciphertext")), + SettingsCertThumbprint: "thumb", + } + + var out map[string]interface{} + err := unmarshalProtectedSettings(cfg, hs, &out) + VerifyErrorClarification(t, constants.Internal_UnmarshalProtectedSettingsFailed, err) +} diff --git a/internal/handlersettings/types.go b/internal/handlersettings/types.go index 49447a6..5033f8c 100644 --- a/internal/handlersettings/types.go +++ b/internal/handlersettings/types.go @@ -1,6 +1,8 @@ package handlersettings import ( + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/pkg/errors" ) @@ -27,13 +29,13 @@ func (s HandlerSettings) ScriptSAS() string { return s.ProtectedSettings.SourceSASToken } -func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, error) { +func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, *vmextension.ErrorWithClarification) { if s.ProtectedSettings.Artifacts == nil && s.PublicSettings.Artifacts == nil { return nil, nil } if len(s.ProtectedSettings.Artifacts) != len(s.PublicSettings.Artifacts) { - return nil, errors.New(("RunCommand artifact download failed. Reason: Invalid artifact specification. This is a product bug.")) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_ArtifactCountMismatch, errors.New(("RunCommand artifact download failed. Reason: Invalid artifact specification. This is a product bug."))) } artifacts := make([]UnifiedArtifact, len(s.PublicSettings.Artifacts)) @@ -57,7 +59,7 @@ func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, error) { } if !found { - return nil, errors.New(("RunCommand artifact download failed. Reason: Invalid artifact specification. This is a product bug.")) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Internal_InvalidArtifactSpecification, errors.New(("RunCommand artifact download failed. Reason: Invalid artifact specification. This is a product bug."))) } } @@ -66,11 +68,11 @@ func (s HandlerSettings) ReadArtifacts() ([]UnifiedArtifact, error) { // validate makes logical validation on the handlerSettings which already passed // the schema validation. -func (s HandlerSettings) validate() error { +func (s HandlerSettings) validate() *vmextension.ErrorWithClarification { // If installAsService is false, then the source has to be specified if !s.PublicSettings.InstallAsService { if s.PublicSettings.Source == nil || (s.PublicSettings.Source.Script == "") == (s.PublicSettings.Source.ScriptURI == "") { - return errSourceNotSpecified + return vmextension.NewErrorWithClarificationPtr(constants.CustomerInput_NoScriptSpecified, errSourceNotSpecified) } } return nil diff --git a/internal/handlersettings/types_test.go b/internal/handlersettings/types_test.go new file mode 100644 index 0000000..d753fb5 --- /dev/null +++ b/internal/handlersettings/types_test.go @@ -0,0 +1,101 @@ +package handlersettings + +import ( + "testing" + + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/stretchr/testify/require" +) + +func TestReadArtifacts_BothNil_ReturnsNilNil(t *testing.T) { + var s HandlerSettings + s.PublicSettings.Artifacts = nil + s.ProtectedSettings.Artifacts = nil + + got, err := s.ReadArtifacts() + require.Nil(t, err) + require.Nil(t, got) +} + +func TestReadArtifacts_CountMismatch_ReturnsArtifactCountMismatch(t *testing.T) { + s := HandlerSettings{ + PublicSettings: PublicSettings{ + Artifacts: []PublicArtifactSource{ + {ArtifactId: 1, ArtifactUri: "https://example/1", FileName: "a.bin"}, + }, + }, + ProtectedSettings: ProtectedSettings{ + Artifacts: []ProtectedArtifactSource{ + {ArtifactId: 1, ArtifactSasToken: "?sig=1"}, + {ArtifactId: 2, ArtifactSasToken: "?sig=2"}, + }, + }, + } + + _, err := s.ReadArtifacts() + VerifyErrorClarification(t, constants.Internal_ArtifactCountMismatch, err) +} + +func TestReadArtifacts_HappyPath_MatchesById_AndPreservesPublicOrder(t *testing.T) { + mi1 := &RunCommandManagedIdentity{ClientId: "client-1"} + mi2 := &RunCommandManagedIdentity{ObjectId: "obj-2"} + + s := HandlerSettings{ + PublicSettings: PublicSettings{ + Artifacts: []PublicArtifactSource{ + {ArtifactId: 10, ArtifactUri: "https://storage/foo", FileName: "foo.txt"}, + {ArtifactId: 20, ArtifactUri: "https://storage/bar", FileName: "bar.txt"}, + }, + }, + ProtectedSettings: ProtectedSettings{ + // Protected list intentionally out of order to ensure matching is by ID, not index. + Artifacts: []ProtectedArtifactSource{ + {ArtifactId: 20, ArtifactSasToken: "?sig=bar", ArtifactManagedIdentity: mi2}, + {ArtifactId: 10, ArtifactSasToken: "?sig=foo", ArtifactManagedIdentity: mi1}, + }, + }, + } + + got, err := s.ReadArtifacts() + require.Nil(t, err) + require.Len(t, got, 2) + + // Must be in the same order as PublicSettings.Artifacts. + require.Equal(t, 10, got[0].ArtifactId) + require.Equal(t, "https://storage/foo", got[0].ArtifactUri) + require.Equal(t, "foo.txt", got[0].FileName) + require.Equal(t, "?sig=foo", got[0].ArtifactSasToken) + require.Same(t, mi1, got[0].ArtifactManagedIdentity) + + require.Equal(t, 20, got[1].ArtifactId) + require.Equal(t, "https://storage/bar", got[1].ArtifactUri) + require.Equal(t, "bar.txt", got[1].FileName) + require.Equal(t, "?sig=bar", got[1].ArtifactSasToken) + require.Same(t, mi2, got[1].ArtifactManagedIdentity) +} + +func TestReadArtifacts_MissingProtectedMatch_ReturnsInvalidArtifactSpecification(t *testing.T) { + s := HandlerSettings{ + PublicSettings: PublicSettings{ + Artifacts: []PublicArtifactSource{ + {ArtifactId: 1, ArtifactUri: "https://storage/a", FileName: "a"}, + {ArtifactId: 2, ArtifactUri: "https://storage/b", FileName: "b"}, + }, + }, + ProtectedSettings: ProtectedSettings{ + Artifacts: []ProtectedArtifactSource{ + {ArtifactId: 1, ArtifactSasToken: "?sig=a"}, + {ArtifactId: 45, ArtifactSasToken: "?sig=b"}, + }, + }, + } + + _, err := s.ReadArtifacts() + VerifyErrorClarification(t, constants.Internal_InvalidArtifactSpecification, err) +} + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/internal/handlersettings/utilities.go b/internal/handlersettings/utilities.go index 6ba375a..4459b73 100644 --- a/internal/handlersettings/utilities.go +++ b/internal/handlersettings/utilities.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/go-kit/kit/log" ) @@ -35,7 +36,7 @@ func GetUriForLogging(uriString string) string { } // Get handler settings from config folder. Example path: /var/lib/waagent/Microsoft.CPlat.Core.RunCommandHandlerLinux-1.3.2/config -func GetHandlerSettings(configFolder string, extensionName string, sequenceNumber int, logContext *log.Context) (HandlerSettings, error) { +func GetHandlerSettings(configFolder string, extensionName string, sequenceNumber int, logContext *log.Context) (HandlerSettings, *vmextension.ErrorWithClarification) { configPath := GetConfigFilePath(configFolder, sequenceNumber, extensionName) cfg, err := ParseAndValidateSettings(logContext, configPath) return cfg, err diff --git a/internal/handlersettings/utilities_test.go b/internal/handlersettings/utilities_test.go new file mode 100644 index 0000000..36e45e9 --- /dev/null +++ b/internal/handlersettings/utilities_test.go @@ -0,0 +1,70 @@ +package handlersettings + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDoesFileExist_FileExists_ReturnsTrue(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "a.txt") + require.NoError(t, os.WriteFile(p, []byte("hi"), 0o600)) + + require.True(t, DoesFileExist(p)) +} + +func TestDoesFileExist_FileMissing_ReturnsFalse(t *testing.T) { + tmp := t.TempDir() + p := filepath.Join(tmp, "missing.txt") + + require.False(t, DoesFileExist(p)) +} + +func TestDoesFileExist_PermissionDenied_ReturnsFalse(t *testing.T) { + // Exercise the branch: err != nil && !IsNotExist(err) => false + tmp := t.TempDir() + + // Create a directory we can't traverse. + noExecDir := filepath.Join(tmp, "noexec") + require.NoError(t, os.Mkdir(noExecDir, 0o600)) // rw------- (no execute) + target := filepath.Join(noExecDir, "secret.txt") + + // Stat will typically fail with EACCES due to missing execute permission on dir. + require.False(t, DoesFileExist(target)) +} + +func TestGetUriForLogging_Empty_ReturnsEmpty(t *testing.T) { + require.Equal(t, "", GetUriForLogging("")) +} + +func TestGetUriForLogging_ValidUrl_StripsQuery(t *testing.T) { + // NOTE: current implementation returns "https//host/path" (missing ':') + in := "https://example.com/container/blob.txt?sv=2020-10-02&sig=abc" + got := GetUriForLogging(in) + require.Equal(t, "https//example.com/container/blob.txt", got) +} + +func TestGetUriForLogging_ValidUrl_NoQuery_UnchangedParts(t *testing.T) { + in := "http://example.com/a/b/c" + got := GetUriForLogging(in) + require.Equal(t, "http//example.com/a/b/c", got) +} + +func TestGetUriForLogging_ParseError_ReturnsEmpty(t *testing.T) { + // Unclosed IPv6 literal => url.Parse fails + in := "http://[::1" + require.Equal(t, "", GetUriForLogging(in)) +} + +func TestGetConfigFilePath_WithExtensionName(t *testing.T) { + got := GetConfigFilePath("/var/lib/waagent/ext/config", 2, "MyExt") + require.Equal(t, filepath.Join("/var/lib/waagent/ext/config", "MyExt.2.settings"), got) +} + +func TestGetConfigFilePath_WithoutExtensionName(t *testing.T) { + got := GetConfigFilePath("/var/lib/waagent/ext/config", 2, "") + require.Equal(t, filepath.Join("/var/lib/waagent/ext/config", "2.settings"), got) +} diff --git a/internal/hostgacommunicator/hostgacommunicator.go b/internal/hostgacommunicator/hostgacommunicator.go index 5d51dcd..8dc7de1 100644 --- a/internal/hostgacommunicator/hostgacommunicator.go +++ b/internal/hostgacommunicator/hostgacommunicator.go @@ -6,6 +6,7 @@ import ( "net/http" "net/url" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/requesthelper" "github.com/go-kit/kit/log" @@ -13,10 +14,16 @@ import ( ) const ( - hostGaPluginPort = "32526" + hostGaPluginPort = "32526" +) + +var ( WireServerFallbackAddress = "http://168.63.129.16:32526" ) +// test seam +var withRetriesFn = requesthelper.WithRetries + type ResponseData struct { VMSettings *VMImmediateExtensionsGoalState ETag string @@ -25,7 +32,7 @@ type ResponseData struct { // Interface for operations available when communicating with HostGAPlugin type IHostGACommunicator interface { - GetImmediateVMSettings(ctx *log.Context, eTag string) (*ResponseData, error) + GetImmediateVMSettings(ctx *log.Context, eTag string) (*ResponseData, *vmextension.ErrorWithClarification) } // HostGaCommunicator provides methods for retrieving VMSettings from the HostGAPlugin @@ -38,19 +45,19 @@ func NewHostGACommunicator(requestManager IVMSettingsRequestManager) HostGACommu } type IVMSettingsRequestManager interface { - GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, error) + GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, *vmextension.ErrorWithClarification) } // GetVMSettings returns the VMSettings for the current machine -func (c *HostGACommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*ResponseData, error) { - requestManager, err := c.vmRequestManager.GetVMSettingsRequestManager(ctx) - if err != nil { - return nil, errors.Wrapf(err, "could not create the request manager to get immediate VMsettings") +func (c *HostGACommunicator) GetImmediateVMSettings(ctx *log.Context, eTag string) (*ResponseData, *vmextension.ErrorWithClarification) { + requestManager, ewc := c.vmRequestManager.GetVMSettingsRequestManager(ctx) + if ewc != nil { + return nil, vmextension.CreateWrappedErrorWithClarification(ewc, "could not create the request manager to get immediate VMsettings") } - resp, err := requesthelper.WithRetries(ctx, requestManager, requesthelper.ActualSleep, eTag) + resp, err := withRetriesFn(ctx, requestManager, requesthelper.ActualSleep, eTag) if err != nil { - return nil, errors.Wrapf(err, "request to retrieve VMSettings failed with retries.") + return nil, vmextension.CreateWrappedErrorWithClarification(err, "request to retrieve VMSettings failed with retries.") } // If the response is 304 Not Modified or 404 Not Found, return nil VMSettings as there are not new goal states to process @@ -66,24 +73,24 @@ func (c *HostGACommunicator) GetImmediateVMSettings(ctx *log.Context, eTag strin var vmSettings VMImmediateExtensionsGoalState if err := json.Unmarshal(body, &vmSettings); err != nil { - return nil, errors.Wrapf(err, "failed to parse immediate VMSettings json") + return nil, vmextension.NewErrorWithClarificationPtr(constants.Hgap_FailedToParseImmediateSettings, errors.Wrapf(err, "failed to parse immediate VMSettings json")) } newETag := resp.Header.Get(constants.ETagHeaderName) if newETag == "" { - return nil, errors.New("ETag not found in response header when retrieving immediate VMSettings") + return nil, vmextension.NewErrorWithClarificationPtr(constants.Hgap_EtagNotFound, errors.New("ETag not found in response header when retrieving immediate VMSettings")) } return &ResponseData{VMSettings: &vmSettings, ETag: newETag, Modified: eTag != newETag}, nil } // Gets the URI to use to call the given operation name -func getOperationUri(ctx *log.Context, operationName string) (string, error) { +func getOperationUri(ctx *log.Context, operationName string) (string, *vmextension.ErrorWithClarification) { // TODO: investigate why other extensions use the env var AZURE_GUEST_AGENT_WIRE_PROTOCOL_ADDRESS // and decide if we want to add that wire protocol address as a potential endpoint to use when provided uri, err := url.Parse(WireServerFallbackAddress) if err != nil { - return "", errors.Wrap(err, "could not parse address "+WireServerFallbackAddress) + return "", vmextension.NewErrorWithClarificationPtr(constants.Hgap_FailedToParseAddress, errors.Wrap(err, "could not parse address "+WireServerFallbackAddress)) } uri.Path = operationName return uri.String(), nil diff --git a/internal/hostgacommunicator/hostgacommunicator_test.go b/internal/hostgacommunicator/hostgacommunicator_test.go index 32b767f..06a8692 100644 --- a/internal/hostgacommunicator/hostgacommunicator_test.go +++ b/internal/hostgacommunicator/hostgacommunicator_test.go @@ -1,10 +1,17 @@ package hostgacommunicator import ( + "bytes" + "io" + "net/http" "os" "testing" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/requesthelper" "github.com/go-kit/kit/log" + "github.com/pkg/errors" "github.com/stretchr/testify/require" ) @@ -16,3 +23,159 @@ func Test_GetOperationUri(t *testing.T) { require.NotNil(t, uri) require.Contains(t, uri, operationName) } + +type fakeVMSettingsRequestManager struct { + rm *requesthelper.RequestManager + err *vmextension.ErrorWithClarification +} + +func (f fakeVMSettingsRequestManager) GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, *vmextension.ErrorWithClarification) { + return f.rm, f.err +} + +func TestGetImmediateVMSettings_RequestManagerError(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + // withRetries should never be called in this branch + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + t.Fatalf("withRetriesFn should not have been called") + return nil, nil + } + + rmErr := vmextension.NewErrorWithClarificationPtr(42, errors.New("the chipmunks have new management")) + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: nil, err: rmErr}) + + _, err := c.GetImmediateVMSettings(nil, "etag0") + VerifyErrorClarification(t, 42, err) +} + +func TestGetImmediateVMSettings_WithRetriesError_WrappedWithClarification(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + return nil, errors.New("network fail") + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + _, err := c.GetImmediateVMSettings(nil, "etag0") + VerifyErrorClarification(t, vmextension.Internal_UnknownError, err) +} + +func TestGetImmediateVMSettings_NotModified304_ReturnsUnmodifiedResponse(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusNotModified, + Body: io.NopCloser(bytes.NewReader(nil)), + Header: make(http.Header), + }, nil + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + resp, err := c.GetImmediateVMSettings(nil, "etag0") + require.Nil(t, err, "unexpected err: %v", err) + require.Nil(t, resp.VMSettings, "expected VMSettings nil") + require.Equal(t, "etag0", resp.ETag, "expected ETag preserved, got %q", resp.ETag) + require.False(t, resp.Modified, "expected Modified=false") +} + +func TestGetImmediateVMSettings_NotFound404_ReturnsUnmodifiedResponse(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusNotFound, + Body: io.NopCloser(bytes.NewReader(nil)), + Header: make(http.Header), + }, nil + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + resp, err := c.GetImmediateVMSettings(nil, "etag0") + require.Nil(t, err, "unexpected err: %v", err) + require.Nil(t, resp.VMSettings, "expected VMSettings nil") + require.Equal(t, "etag0", resp.ETag, "expected ETag preserved, got %q", resp.ETag) + require.False(t, resp.Modified, "expected Modified=false") +} + +func TestGetImmediateVMSettings_BadJSON_ReturnsFailedToParseSettings(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + h := make(http.Header) + h.Set(constants.ETagHeaderName, "etag1") // still present, but parse should fail first + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte("{not-json"))), + Header: h, + }, nil + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + _, err := c.GetImmediateVMSettings(nil, "etag0") + VerifyErrorClarification(t, constants.Hgap_FailedToParseImmediateSettings, err) +} + +func TestGetImmediateVMSettings_MissingETagHeader_ReturnsEtagNotFoundClarification(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + // minimal valid JSON for VMImmediateExtensionsGoalState; if required fields exist, update accordingly. + body := []byte(`{}`) + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(body)), + Header: make(http.Header), // no ETag set + }, nil + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + _, err := c.GetImmediateVMSettings(nil, "etag0") + VerifyErrorClarification(t, constants.Hgap_EtagNotFound, err) +} + +func TestGetImmediateVMSettings_Success_ModifiedFlagAndETagReturned(t *testing.T) { + orig := withRetriesFn + t.Cleanup(func() { withRetriesFn = orig }) + + withRetriesFn = func(_ *log.Context, _ *requesthelper.RequestManager, _ requesthelper.SleepFunc, _ string) (*http.Response, error) { + h := make(http.Header) + h.Set(constants.ETagHeaderName, "etag1") + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader([]byte(`{}`))), + Header: h, + }, nil + } + + c := NewHostGACommunicator(fakeVMSettingsRequestManager{rm: &requesthelper.RequestManager{}, err: nil}) + + resp, err := c.GetImmediateVMSettings(nil, "etag0") + require.Nil(t, err, "unexpected err: %v", err) + require.NotNil(t, resp.VMSettings, "expected VMSettings non-nil") + require.Equal(t, "etag1", resp.ETag, "expected etag1 preserved, got %q", resp.ETag) + require.True(t, resp.Modified, "expected Modified=true when etag changes") +} + +func TestGetOperationUri_InvalidFallbackAddress(t *testing.T) { + orig := WireServerFallbackAddress + t.Cleanup(func() { WireServerFallbackAddress = orig }) + + // This should make url.Parse fail (unclosed IPv6 literal). + WireServerFallbackAddress = "http://[::1:32526" + + _, err := getOperationUri(nil, "/machine") + VerifyErrorClarification(t, constants.Hgap_FailedToParseAddress, err) +} diff --git a/internal/hostgacommunicator/vmsettings.go b/internal/hostgacommunicator/vmsettings.go index 8d5af31..cab5135 100644 --- a/internal/hostgacommunicator/vmsettings.go +++ b/internal/hostgacommunicator/vmsettings.go @@ -7,6 +7,7 @@ import ( "path/filepath" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" requesthelper "github.com/Azure/run-command-handler-linux/internal/requesthelper" @@ -23,6 +24,10 @@ const ( vmSettingsRequestTimeout = 30 * time.Second ) +var ( + getHandlerEnvFn = handlersettings.GetHandlerEnv +) + type VMImmediateExtensionsGoalState struct { ImmediateExtensionGoalStates []ImmediateExtensionGoalState `json:"immediateExtensionsGoalStates"` } @@ -38,10 +43,10 @@ type requestFactory struct { } // Returns a new RequestManager object useful to make GET Requests -func GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, error) { +func GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, *vmextension.ErrorWithClarification) { factory, err := newVMSettingsRequestFactory(ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to create request factory") + return nil, vmextension.NewErrorWithClarificationPtr(constants.Hgap_FailedToCreateRequestFactory, errors.Wrapf(err, "failed to create request factory")) } return requesthelper.GetRequestManager(factory, vmSettingsRequestTimeout), nil @@ -51,7 +56,7 @@ func GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManage func newVMSettingsRequestFactory(ctx *log.Context) (*requestFactory, error) { url, err := getOperationUri(ctx, vmSettingsOperation) if err != nil { - return nil, errors.Wrapf(err, "failed to obtain VMSettingsURI") + return nil, vmextension.CreateWrappedErrorWithClarification(err, "failed to obtain VMSettingsURI") } return &requestFactory{url}, nil @@ -62,19 +67,19 @@ func (u requestFactory) GetRequest(ctx *log.Context, eTag string) (*http.Request request, err := http.NewRequest("GET", u.url, nil) if err != nil { errMsg := fmt.Sprintf("failed to create request to %v", u.url) - return nil, errors.Wrap(err, errMsg) + return nil, vmextension.NewErrorWithClarificationPtr(constants.Hgap_FailedCreateRequest, errors.Wrap(err, errMsg)) } if eTag != "" { request.Header.Set(constants.IfNoneMatchHeaderName, eTag) } - return request, err + return request, nil } -func (goalState *ImmediateExtensionGoalState) ValidateSignature() (bool, error) { - he, err := handlersettings.GetHandlerEnv() +func (goalState *ImmediateExtensionGoalState) ValidateSignature() (bool, *vmextension.ErrorWithClarification) { + he, err := getHandlerEnvFn() if err != nil { - return false, errors.Wrap(err, "failed to parse handlerenv") + return false, err } configFolder := he.HandlerEnvironment.ConfigFolder @@ -86,7 +91,7 @@ func (goalState *ImmediateExtensionGoalState) ValidateSignature() (bool, error) } if s.SettingsCertThumbprint == "" { - return false, errors.New("HandlerSettings has protected settings but no cert thumbprint") + return false, vmextension.NewErrorWithClarificationPtr(constants.Hgap_NoCertThumbprint, errors.New("HandlerSettings has protected settings but no cert thumbprint")) } // go two levels up where certs are placed (/var/lib/waagent) @@ -95,7 +100,7 @@ func (goalState *ImmediateExtensionGoalState) ValidateSignature() (bool, error) if !fileExists(crt) || !fileExists(prv) { message := fmt.Sprintf("Certificate %v needed by %v is missing from the goal state", s.SettingsCertThumbprint, s.ExtensionName) - return false, errors.New(message) + return false, vmextension.NewErrorWithClarificationPtr(constants.Hgap_CertificateMissingFromGoalState, errors.New(message)) } } diff --git a/internal/hostgacommunicator/vmsettings_test.go b/internal/hostgacommunicator/vmsettings_test.go index 8a8570f..5415cde 100644 --- a/internal/hostgacommunicator/vmsettings_test.go +++ b/internal/hostgacommunicator/vmsettings_test.go @@ -1,13 +1,18 @@ package hostgacommunicator import ( + "errors" "net/http" "net/http/httptest" "os" "path" "testing" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/requesthelper" + "github.com/Azure/run-command-handler-linux/internal/settings" + "github.com/Azure/run-command-handler-linux/internal/types" "github.com/ahmetb/go-httpbin" "github.com/go-kit/kit/log" "github.com/stretchr/testify/require" @@ -29,7 +34,7 @@ type TestRequestManager struct { testUrlRequest *TestUrlRequest } -func (li *TestRequestManager) GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, error) { +func (li *TestRequestManager) GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, *vmextension.ErrorWithClarification) { return requesthelper.GetRequestManager(li.testUrlRequest, vmSettingsRequestTimeout), nil } @@ -45,6 +50,33 @@ func Test_GetImmediateVMSettingsFailedToParseJson(t *testing.T) { _, err := communicator.GetImmediateVMSettings(ctx, "") require.NotNil(t, err) require.ErrorContains(t, err, "failed to parse immediate VMSettings json") + VerifyErrorClarification(t, constants.Hgap_FailedToParseImmediateSettings, err) +} + +func TestRequestFactory_GetRequest_InvalidURL_ReturnsErrorWithClarification(t *testing.T) { + // Intentionally malformed URL to force http.NewRequest to fail. + f := requestFactory{url: "http://[::1"} // invalid host bracket + + _, err := f.GetRequest(nil, "") + if err == nil { + t.Fatalf("expected error, got nil") + } + + var ewc *vmextension.ErrorWithClarification + require.True(t, errors.As(err, &ewc), "Error is not of type ErrorWithClarification") + VerifyErrorClarification(t, constants.Hgap_FailedCreateRequest, ewc) +} + +func Test_GetVMSettingsRequestManager_CannotParseUri(t *testing.T) { + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + + wsAddress := WireServerFallbackAddress + defer func() { WireServerFallbackAddress = wsAddress }() + WireServerFallbackAddress = ":invalid_chipmunk" + + requestManager, err := GetVMSettingsRequestManager(ctx) + require.Nil(t, requestManager) + VerifyErrorClarification(t, constants.Hgap_FailedToCreateRequestFactory, err) } func Test_GetImmediateVMSettingsHandleNotFound(t *testing.T) { @@ -60,6 +92,69 @@ func Test_GetImmediateVMSettingsHandleNotFound(t *testing.T) { require.Nil(t, err, "should not return error as this means no new goal states to process") } +func TestValidateSignature_CertMissingFromGoalState(t *testing.T) { + thumb := "abc123" + extensionName := "noncertchipmunk" + + he := types.HandlerEnvironment{ + Version: 1.0, + Name: "ExampleExtension", + } + he.HandlerEnvironment.ConfigFolder = "blah" + + orig := getHandlerEnvFn + defer func() { getHandlerEnvFn = orig }() + getHandlerEnvFn = func() (types.HandlerEnvironment, *vmextension.ErrorWithClarification) { + return he, nil + } + + gs := &ImmediateExtensionGoalState{ + Name: "test", + Settings: []settings.SettingsCommon{ + { + ExtensionName: &extensionName, + ProtectedSettingsBase64: "not-empty", + SettingsCertThumbprint: thumb, + }, + }, + } + + ok, err := gs.ValidateSignature() + require.False(t, ok, "Received success when failure expected") + VerifyErrorClarification(t, constants.Hgap_CertificateMissingFromGoalState, err) +} + +func TestValidateSignature_NoCertThumbprint(t *testing.T) { + extensionName := "noncertchipmunk" + + he := types.HandlerEnvironment{ + Version: 1.0, + Name: "ExampleExtension", + } + he.HandlerEnvironment.ConfigFolder = "blah" + + orig := getHandlerEnvFn + defer func() { getHandlerEnvFn = orig }() + getHandlerEnvFn = func() (types.HandlerEnvironment, *vmextension.ErrorWithClarification) { + return he, nil + } + + gs := &ImmediateExtensionGoalState{ + Name: "test", + Settings: []settings.SettingsCommon{ + { + ExtensionName: &extensionName, + ProtectedSettingsBase64: "not-empty", + SettingsCertThumbprint: "", + }, + }, + } + + ok, err := gs.ValidateSignature() + require.False(t, ok, "Received success when failure expected") + VerifyErrorClarification(t, constants.Hgap_NoCertThumbprint, err) +} + func Test_GetVMSettingsRequestManager(t *testing.T) { ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) requestManager, err := GetVMSettingsRequestManager(ctx) @@ -80,3 +175,35 @@ func Test_FileExists(t *testing.T) { require.False(t, fileExists(nonExistentFile)) require.True(t, fileExists(existentFile)) } + +func TestRequestFactory_GetRequest_SetsIfNoneMatchWhenProvided(t *testing.T) { + f := requestFactory{url: "http://example.com/foo"} + + req, err := f.GetRequest(nil, "etag-123") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if req.Method != "GET" { + t.Fatalf("expected GET, got %q", req.Method) + } + if got := req.Header.Get(constants.IfNoneMatchHeaderName); got != "etag-123" { + t.Fatalf("expected If-None-Match %q, got %q", "etag-123", got) + } +} + +func TestRequestFactory_GetRequest_DoesNotSetIfNoneMatchWhenEmpty(t *testing.T) { + f := requestFactory{url: "http://example.com/foo"} + + req, err := f.GetRequest(nil, "") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got := req.Header.Get(constants.IfNoneMatchHeaderName); got != "" { + t.Fatalf("expected If-None-Match to be empty, got %q", got) + } +} + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/internal/immediatecmds/immediatecmds.go b/internal/immediatecmds/immediatecmds.go index 378eb7f..1b2372f 100644 --- a/internal/immediatecmds/immediatecmds.go +++ b/internal/immediatecmds/immediatecmds.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/Azure/azure-extension-platform/pkg/extensionevents" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/handlersettings" "github.com/Azure/run-command-handler-linux/internal/service" @@ -12,23 +13,33 @@ import ( "github.com/pkg/errors" ) +var ( + fnServiceDisable = service.Disable + fnServiceDeRegister = service.DeRegister + fnServiceEnable = service.Enable + fnServiceIsEnabled = service.IsEnabled + fnServiceIsInstalled = service.IsInstalled + fnServiceRegister = service.Register + fnServiceStart = service.Start +) + // Updates the service definition if any immediate run command service exists. // The action is skipped if the service has already been upgraded. func Update(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNum int, extensionEvents *extensionevents.ExtensionEventManager) (int, error) { ctx.Log("message", "updating immediate run command") - isInstalled, err := service.IsInstalled(ctx) + isInstalled, err := fnServiceIsInstalled(ctx) if err != nil { errMessage := fmt.Sprintf("Failed to check if any runcommand service is installed: %v", err) extensionEvents.LogErrorEvent("immediateupdate", errMessage) - return constants.ExitCode_CreateDataDirectoryFailed, errors.Wrap(err, "failed to check if any runcommand service is installed") + return constants.FileSystem_CreateDataDirectoryFailed, errors.Wrap(err, "failed to check if any runcommand service is installed") } if isInstalled { - err = service.Register(ctx, extensionEvents) - if err != nil { - errMessage := fmt.Sprintf("Failed to upgrade run command service: %v", err) + ewc := fnServiceRegister(ctx, extensionEvents) + if ewc != nil { + errMessage := fmt.Sprintf("Failed to upgrade run command service: %v", ewc) extensionEvents.LogErrorEvent("immediateupdate", errMessage) - return constants.ExitCode_UpgradeInstalledServiceFailed, errors.Wrap(err, "failed to upgrade run command service") + return constants.ExitCode_UpgradeInstalledServiceFailed, vmextension.CreateWrappedErrorWithClarification(ewc, "failed to upgrade run command service") } } @@ -36,7 +47,7 @@ func Update(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNum } func Disable(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNum int, extensionEvents *extensionevents.ExtensionEventManager) (int, error) { - isInstalled, err := service.IsInstalled(ctx) + isInstalled, err := fnServiceIsInstalled(ctx) if err != nil { errMessage := fmt.Sprintf("Failed to check if runcommand service is installed: %v", err) extensionEvents.LogErrorEvent("immediatedisable", errMessage) @@ -44,7 +55,7 @@ func Disable(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNu } if isInstalled { - isEnabled, err := service.IsEnabled(ctx) + isEnabled, err := fnServiceIsEnabled(ctx) if err != nil { errMessage := fmt.Sprintf("Failed to check if service is enabled: %v", err) extensionEvents.LogErrorEvent("immediatedisable", errMessage) @@ -52,7 +63,7 @@ func Disable(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNu } if isEnabled { - err := service.Disable(ctx, extensionEvents) + err := fnServiceDisable(ctx, extensionEvents) if err != nil { errMessage := fmt.Sprintf("Failed to disable run command service: %v", err) extensionEvents.LogErrorEvent("immediatedisable", errMessage) @@ -74,19 +85,19 @@ func Install() (int, error) { func Uninstall(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNum int, extensionEvents *extensionevents.ExtensionEventManager) (int, error) { ctx.Log("message", "proceeding to uninstall immediate run command") - isInstalled, err := service.IsInstalled(ctx) + isInstalled, err := fnServiceIsInstalled(ctx) if err != nil { errMessage := fmt.Sprintf("Failed to check if runcommand service is installed: %v", err) extensionEvents.LogErrorEvent("immediatedisable", errMessage) - return constants.ExitCode_RemoveDataDirectoryFailed, errors.Wrap(err, "failed to check if runcommand service is installed") + return constants.FileSystem_RemoveDataDirectoryFailed, errors.Wrap(err, "failed to check if runcommand service is installed") } if isInstalled { - error := service.DeRegister(ctx, extensionEvents) - if error != nil { - errMessage := fmt.Sprintf("Failed to uninstall run command service: %v", error) + err2 := fnServiceDeRegister(ctx, extensionEvents) + if err2 != nil { + errMessage := fmt.Sprintf("Failed to uninstall run command service: %v", err2) extensionEvents.LogErrorEvent("immediatedisable", errMessage) - return constants.ExitCode_UninstallInstalledServiceFailed, errors.Wrap(err, "failed to uninstall run command service") + return constants.ExitCode_UninstallInstalledServiceFailed, errors.Wrap(err2, "failed to uninstall run command service") } } return constants.ExitCode_Okay, nil @@ -95,42 +106,42 @@ func Uninstall(ctx *log.Context, h types.HandlerEnvironment, extName string, seq func Enable(ctx *log.Context, h types.HandlerEnvironment, extName string, seqNum int, cfg handlersettings.HandlerSettings, extensionEvents *extensionevents.ExtensionEventManager) (int, error) { // If installService == true, then install RunCommand as a service if cfg.InstallAsService() { - isInstalled, err2 := service.IsInstalled(ctx) + isInstalled, err2 := fnServiceIsInstalled(ctx) if err2 != nil { ctx.Log("message", "could not check if service is already installed. Proceeding to overwrite configuration file to make sure it gets installed.") extensionEvents.LogErrorEvent("immediateenable", "could not check if service is already installed. Proceeding to overwrite configuration file to make sure it gets installed.") } if !isInstalled { - err3 := service.Register(ctx, extensionEvents) + err3 := fnServiceRegister(ctx, extensionEvents) if err3 != nil { errMessage := fmt.Sprintf("Failed to install RunCommand as a service: %v", err3) extensionEvents.LogErrorEvent("immediateenable", errMessage) - return constants.ExitCode_InstallServiceFailed, errors.Wrap(err3, "failed to install RunCommand as a service") + return constants.Immediate_CouldNotStartService, err3 } } else { - isEnabled, err3 := service.IsEnabled(ctx) + isEnabled, err3 := fnServiceIsEnabled(ctx) if err3 != nil { errMessage := fmt.Sprintf("Failed to check if service is already enabled: %v", err3) extensionEvents.LogErrorEvent("immediateenable", errMessage) - return constants.ExitCode_InstallServiceFailed, errors.Wrap(err3, "failed to check if service is already enabled") + return constants.Immediate_CouldNotCheckServiceAlreadyEnabled, vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotCheckServiceAlreadyEnabled, errors.Wrap(err3, errMessage)) } if !isEnabled { - err4 := service.Enable(ctx, extensionEvents) + err4 := fnServiceEnable(ctx, extensionEvents) if err4 != nil { errMessage := fmt.Sprintf("Failed to enable service: %v", err4) extensionEvents.LogErrorEvent("immediateenable", errMessage) - return constants.ExitCode_InstallServiceFailed, errors.Wrap(err4, "failed to enable service") + return constants.Immediate_EnableServiceFailed, vmextension.NewErrorWithClarificationPtr(constants.Immediate_EnableServiceFailed, errors.Wrap(err4, errMessage)) } - err5 := service.Start(ctx, extensionEvents) + err5 := fnServiceStart(ctx, extensionEvents) if err5 != nil { errMessage := fmt.Sprintf("Failed to start service: %v", err5) extensionEvents.LogErrorEvent("immediateenable", errMessage) - return constants.ExitCode_InstallServiceFailed, errors.Wrap(err5, "failed to start service") + return constants.Immediate_CouldNotStartService, vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotStartService, errors.Wrap(err5, errMessage)) } } } diff --git a/internal/immediatecmds/immediatecmds_test.go b/internal/immediatecmds/immediatecmds_test.go new file mode 100644 index 0000000..4820125 --- /dev/null +++ b/internal/immediatecmds/immediatecmds_test.go @@ -0,0 +1,527 @@ +package immediatecmds + +import ( + "errors" + "os" + "testing" + + "github.com/Azure/azure-extension-platform/pkg/extensionevents" + "github.com/Azure/azure-extension-platform/pkg/handlerenv" + "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/handlersettings" + "github.com/Azure/run-command-handler-linux/internal/types" + "github.com/go-kit/kit/log" + "github.com/stretchr/testify/require" +) + +// We keep originals so we can restore after each test. +var ( + origIsInstalled = fnServiceIsInstalled + origIsEnabled = fnServiceIsEnabled + origRegister = fnServiceRegister + origDisable = fnServiceDisable + origEnable = fnServiceEnable + origStart = fnServiceStart + origDeRegister = fnServiceDeRegister +) + +func restoreServiceFns() { + fnServiceIsInstalled = origIsInstalled + fnServiceIsEnabled = origIsEnabled + fnServiceRegister = origRegister + fnServiceDisable = origDisable + fnServiceEnable = origEnable + fnServiceStart = origStart + fnServiceDeRegister = origDeRegister +} + +func TestUpdate_IsInstalledError(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { + return false, errors.New("boom") + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "IsInstalledError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Update(ctx, types.HandlerEnvironment{}, "ext", 5, events) + + require.Error(t, err) + require.Equal(t, constants.FileSystem_CreateDataDirectoryFailed, code) +} + +func TestUpdate_Installed_RegisterFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { + return vmextension.NewErrorWithClarificationPtr(42, errors.New("register failure")) + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "RegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Update(ctx, types.HandlerEnvironment{}, "ext", 5, events) + require.Error(t, err) + require.Equal(t, constants.ExitCode_UpgradeInstalledServiceFailed, code) +} + +func TestUpdate_Success_NoUpgradeNeeded(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "NoUpgradeNeeded") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Update(ctx, types.HandlerEnvironment{}, "ext", 5, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestUpdate_Success_UpgradeService(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { + return nil + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "UpgradeService") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Update(ctx, types.HandlerEnvironment{}, "ext", 5, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestDisable_ErrorCheckingInstalled(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, errors.New("fail") } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "ErrorCheckingInstalled") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Disable(ctx, types.HandlerEnvironment{}, "ext", 1, events) + require.Error(t, err) + require.Equal(t, constants.ExitCode_DisableInstalledServiceFailed, code) +} + +func TestDisable_Installed_ErrorCheckingEnabled(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return false, errors.New("fail") } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "ErrorCheckingEnabled") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Disable(ctx, types.HandlerEnvironment{}, "ext", 1, events) + require.Error(t, err) + require.Equal(t, constants.ExitCode_InstallServiceFailed, code) +} + +func TestDisable_Installed_Enabled_DisableFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceDisable = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { + return errors.New("disable err") + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "DisableFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Disable(ctx, types.HandlerEnvironment{}, "ext", 1, events) + require.Error(t, err) + require.Equal(t, constants.ExitCode_DisableInstalledServiceFailed, code) +} + +func TestDisable_Installed_AlreadyDisabled(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return false, nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "AlreadyDisabled") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Disable(ctx, types.HandlerEnvironment{}, "ext", 1, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestDisable_Installed_DisableSuccess(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceDisable = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { return nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "DisableSuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Disable(ctx, types.HandlerEnvironment{}, "ext", 1, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestInstall_AlwaysOkay(t *testing.T) { + code, err := Install() + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestUninstall_CheckInstalledError(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, errors.New("fail") } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "CheckInstalledError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Uninstall(ctx, types.HandlerEnvironment{}, "x", 2, events) + require.Error(t, err) + require.Equal(t, constants.FileSystem_RemoveDataDirectoryFailed, code) +} + +func TestUninstall_Installed_DeRegisterFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceDeRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { + return errors.New("the chipmunks do not register") + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "DeRegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Uninstall(ctx, types.HandlerEnvironment{}, "x", 2, events) + require.Error(t, err) + require.Equal(t, constants.ExitCode_UninstallInstalledServiceFailed, code) +} + +func TestUninstall_Installed_DeRegisterSuccess(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceDeRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { return nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "deregistersuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Uninstall(ctx, types.HandlerEnvironment{}, "x", 2, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestUninstall_NotInstalled(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "notinstalled") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Uninstall(ctx, types.HandlerEnvironment{}, "x", 2, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestEnable_NoServiceInstallRequested(t *testing.T) { + cfg := handlersettings.HandlerSettings{} + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "noserviceinstallrequested") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestEnable_Install_CheckInstalledFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, errors.New("check failed") } + fnServiceRegister = func(ctx *log.Context, extensionEvents *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { + return nil + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "checkinstallfails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.NoError(t, err) // Notice: Enable ignores this error; only logs + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestEnable_Install_NotInstalled_RegisterFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, nil } + fnServiceRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { + return vmextension.NewErrorWithClarificationPtr(42, errors.New("reg fail")) + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "registerfails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.Error(t, err) + require.Equal(t, constants.Immediate_CouldNotStartService, code) +} + +func TestEnable_Install_NotInstalled_RegisterSuccess(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return false, nil } + fnServiceRegister = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { + return nil + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "registersuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func TestEnable_Install_Installed_CheckEnabledFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return false, errors.New("oops") } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "checkenabledfails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.Error(t, err) + require.Equal(t, constants.Immediate_CouldNotCheckServiceAlreadyEnabled, code) + VerifyErrorClarification(t, constants.Immediate_CouldNotCheckServiceAlreadyEnabled, err) +} + +func TestEnable_Install_Installed_NotEnabled_EnableFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return false, nil } + fnServiceEnable = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { + return errors.New("enablefail") + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "enablefails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.Error(t, err) + require.Equal(t, constants.Immediate_EnableServiceFailed, code) +} + +func TestEnable_Install_Installed_NotEnabled_StartFails(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return false, nil } + fnServiceEnable = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { return nil } + fnServiceStart = func(ctx *log.Context, ev *extensionevents.ExtensionEventManager) error { + return errors.New("startfail") + } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "startfails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.Error(t, err) + require.Equal(t, constants.Immediate_CouldNotStartService, code) +} + +func TestEnable_Install_Installed_Enabled_NoOp(t *testing.T) { + restoreServiceFns() + defer restoreServiceFns() + + cfg := getInstallAsServiceCfg() + + fnServiceIsInstalled = func(ctx *log.Context) (bool, error) { return true, nil } + fnServiceIsEnabled = func(ctx *log.Context) (bool, error) { return true, nil } + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "noop") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + extensionLogger := logging.New(nil) + events := extensionevents.New(extensionLogger, &handlerEnvironment) + + code, err := Enable(ctx, types.HandlerEnvironment{}, "ext", 1, cfg, events) + require.NoError(t, err) + require.Equal(t, constants.ExitCode_Okay, code) +} + +func getInstallAsServiceCfg() handlersettings.HandlerSettings { + cfg := handlersettings.HandlerSettings{} + cfg.PublicSettings.InstallAsService = true + return cfg +} + +func VerifyErrorClarification(t *testing.T, expectedCode int, err error) { + require.NotNil(t, err, "No error returned when one was expected") + var ewc *vmextension.ErrorWithClarification + require.True(t, errors.As(err, &ewc), "Error is not of type ErrorWithClarification") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/internal/immediateruncommand/immediateruncommand.go b/internal/immediateruncommand/immediateruncommand.go index 5725ed9..fb9f4d4 100644 --- a/internal/immediateruncommand/immediateruncommand.go +++ b/internal/immediateruncommand/immediateruncommand.go @@ -5,6 +5,7 @@ import ( "math" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/internal/goalstate" "github.com/Azure/run-command-handler-linux/internal/hostgacommunicator" @@ -15,13 +16,30 @@ import ( "github.com/Azure/run-command-handler-linux/internal/types" "github.com/Azure/run-command-handler-linux/pkg/counterutil" "github.com/go-kit/kit/log" - "github.com/pkg/errors" ) const ( maxConcurrentTasks int32 = 5 ) +// ---- test seams (override in *_test.go) ---- +var ( + getImmediateGoalStatesFn = goalstate.GetImmediateRunCommandGoalStates + handleImmediateGoalStateFn = goalstate.HandleImmediateGoalState + reportFinalStatusFn = goalstate.ReportFinalStatusForImmediateGoalState + + // signature validation seam (lets tests bypass crypto fields on ImmediateExtensionGoalState) + validateSignatureFn = func(el hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { + return el.ValidateSignature() + } + + // goroutine seam (lets tests run synchronously) + spawnFn = func(f func()) { go f() } + + // time seam + nowFn = func() time.Time { return time.Now().UTC() } +) + var executingTasks counterutil.AtomicCount // goalStateEventObserver is an observer that listens for status changes in goal states. @@ -31,11 +49,11 @@ var goalStateEventObserver = status.StatusObserver{} type VMSettingsRequestManager struct{} -func (*VMSettingsRequestManager) GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, error) { +func (*VMSettingsRequestManager) GetVMSettingsRequestManager(ctx *log.Context) (*requesthelper.RequestManager, *vmextension.ErrorWithClarification) { return hostgacommunicator.GetVMSettingsRequestManager(ctx) } -func StartImmediateRunCommand(ctx *log.Context) error { +func StartImmediateRunCommand(ctx *log.Context) *vmextension.ErrorWithClarification { ctx.Log("message", "starting immediate run command service") var vmRequestManager = new(VMSettingsRequestManager) var lastProcessedETag string = "" @@ -47,7 +65,7 @@ func StartImmediateRunCommand(ctx *log.Context) error { newProcessedETag, err := processImmediateRunCommandGoalStates(ctx, communicator, lastProcessedETag) if err != nil { - ctx.Log("error", errors.Wrapf(err, "could not process new immediate run command states because of an unexpected error")) + ctx.Log("error", vmextension.CreateWrappedErrorWithClarification(err, "could not process new immediate run command states because of an unexpected error")) ctx.Log("message", "sleep for 5 seconds before retrying") time.Sleep(time.Second * time.Duration(5)) } else { @@ -61,7 +79,7 @@ func StartImmediateRunCommand(ctx *log.Context) error { } } -func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgacommunicator.HostGACommunicator, lastProcessedETag string) (string, error) { +func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgacommunicator.HostGACommunicator, lastProcessedETag string) (string, *vmextension.ErrorWithClarification) { executingTaskCount := executingTasks.Get() maxTasksToFetch := int(math.Max(float64(maxConcurrentTasks-executingTaskCount), 0)) @@ -74,9 +92,9 @@ func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgac return lastProcessedETag, nil } - goalStates, newEtag, err := goalstate.GetImmediateRunCommandGoalStates(ctx, &communicator, lastProcessedETag) + goalStates, newEtag, err := getImmediateGoalStatesFn(ctx, &communicator, lastProcessedETag) if err != nil { - return newEtag, errors.Wrapf(err, "could not retrieve goal states for immediate run command") + return newEtag, vmextension.CreateWrappedErrorWithClarification(err, "could not retrieve goal states for immediate run command") } // VM Settings have not changed and we should not process any new goal states @@ -93,16 +111,18 @@ func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgac } } goalStateEventObserver.RemoveProcessedGoalStates(goalStateKeys) - newGoalStates, skippedGoalStates, err := getGoalStatesToProcess(goalStates, maxTasksToFetch) - if err != nil { - return newEtag, errors.Wrap(err, "could not get goal states to process") + newGoalStates, skippedGoalStates, ewc := getGoalStatesToProcess(goalStates, maxTasksToFetch) + if ewc != nil { + return newEtag, vmextension.CreateWrappedErrorWithClarification(err, "could not get goal states to process") } if len(newGoalStates) > 0 { ctx.Log("message", fmt.Sprintf("trying to launch %v goal states concurrently", len(newGoalStates))) for idx := range newGoalStates { - go func(state settings.SettingsCommon) { + st := newGoalStates[idx] + spawnFn(func() { + state := st ctx.Log("message", "launching new goal state. Incrementing executing tasks counter") executingTasks.Increment() @@ -114,29 +134,33 @@ func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgac notifier := &observer.Notifier{} notifier.Register(&goalStateEventObserver) notifier.Notify(status) - startTime := time.Now().UTC().Format(time.RFC3339) - exitCode, err := goalstate.HandleImmediateGoalState(ctx, state, notifier) + startTime := nowFn().Format(time.RFC3339) + exitCode, ewc := handleImmediateGoalStateFn(ctx, state, notifier) ctx.Log("message", "goal state has exited. Decrementing executing tasks counter") executingTasks.Decrement() // If there was an error executing the goal state, report the final status to the HGAP // For successful goal states, the status is reported by the usual workflow - if err != nil { - ctx.Log("error", "failed to execute goal state", "message", err) + if ewc != nil { + ctx.Log("error", "failed to execute goal state", "message", ewc) + + errorCode := ewc.ErrorCode + instView := types.RunCommandInstanceView{ - ExecutionState: types.Failed, - ExecutionMessage: "Execution failed", - ExitCode: exitCode, - Output: "", - Error: err.Error(), - StartTime: startTime, - EndTime: time.Now().UTC().Format(time.RFC3339), + ExecutionState: types.Failed, + ExecutionMessage: "Execution failed", + ExitCode: exitCode, + Output: "", + Error: ewc.Error(), + StartTime: startTime, + EndTime: nowFn().Format(time.RFC3339), + ErrorClarificationValue: errorCode, } - goalstate.ReportFinalStatusForImmediateGoalState(ctx, notifier, statusKey, types.StatusError, &instView) + reportFinalStatusFn(ctx, notifier, statusKey, types.StatusError, &instView) } - }(newGoalStates[idx]) + }) } ctx.Log("message", "finished launching goal states") @@ -155,13 +179,13 @@ func processImmediateRunCommandGoalStates(ctx *log.Context, communicator hostgac instView := types.RunCommandInstanceView{ ExecutionState: types.Failed, ExecutionMessage: "Execution was skipped due to reaching the maximum concurrent tasks", - ExitCode: constants.ExitCode_SkippedImmediateGoalState, + ExitCode: constants.ImmediateRC_CommandSkipped, Output: "", Error: errorMsg, StartTime: time.Now().UTC().Format(time.RFC3339), EndTime: time.Now().UTC().Format(time.RFC3339), } - goalstate.ReportFinalStatusForImmediateGoalState(ctx, notifier, statusKey, types.StatusSkipped, &instView) + reportFinalStatusFn(ctx, notifier, statusKey, types.StatusSkipped, &instView) } } else { ctx.Log("message", "no goal states were skipped") @@ -175,9 +199,9 @@ func getGoalStatesToProcess(goalStates []hostgacommunicator.ImmediateExtensionGo var newGoalStates []settings.SettingsCommon var skippedGoalStates []settings.SettingsCommon for _, el := range goalStates { - validSignature, err := el.ValidateSignature() + validSignature, err := validateSignatureFn(el) if err != nil { - return nil, nil, errors.Wrap(err, "failed to validate goal state signature") + return nil, nil, err } if validSignature { diff --git a/internal/immediateruncommand/immediateruncommand_test.go b/internal/immediateruncommand/immediateruncommand_test.go new file mode 100644 index 0000000..b741340 --- /dev/null +++ b/internal/immediateruncommand/immediateruncommand_test.go @@ -0,0 +1,295 @@ +package immediateruncommand + +import ( + "errors" + "os" + "testing" + "time" + + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/internal/hostgacommunicator" + "github.com/Azure/run-command-handler-linux/internal/observer" + "github.com/Azure/run-command-handler-linux/internal/settings" + "github.com/Azure/run-command-handler-linux/internal/status" + "github.com/Azure/run-command-handler-linux/internal/types" + "github.com/go-kit/kit/log" + "github.com/stretchr/testify/require" +) + +// ---- helpers ---- + +func ptrString(s string) *string { return &s } +func ptrInt(i int) *int { return &i } +func ptrInt32(i int32) *int32 { return &i } + +func mkSetting(ext string, seq int, state string) settings.SettingsCommon { + extCopy := ext + seqCopy := seq + stateCopy := state + return settings.SettingsCommon{ + ExtensionName: &extCopy, + SeqNo: &seqCopy, + ExtensionState: &stateCopy, + } +} + +// ---- tests for getGoalStatesToProcess ---- + +func TestGetGoalStatesToProcess_ValidateSignatureError(t *testing.T) { + orig := validateSignatureFn + defer func() { validateSignatureFn = orig }() + validateSignatureFn = func(_ hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { + return false, errors.New("boom") + } + + gs := []hostgacommunicator.ImmediateExtensionGoalState{ + {Settings: []settings.SettingsCommon{mkSetting("RunCommand", 1, "state")}}, + } + + _, _, err := getGoalStatesToProcess(gs, 10) + require.NotNil(t, err, "expected error, got nil") +} + +func TestGetGoalStatesToProcess_InvalidSignatureSkipsAll(t *testing.T) { + orig := validateSignatureFn + defer func() { validateSignatureFn = orig }() + validateSignatureFn = func(_ hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { + return false, nil + } + + gs := []hostgacommunicator.ImmediateExtensionGoalState{ + {Settings: []settings.SettingsCommon{ + mkSetting("RunCommand", 1, "A"), + mkSetting("RunCommand", 2, "B"), + }}, + } + + newOnes, skipped, err := getGoalStatesToProcess(gs, 10) + require.Nil(t, err, "unexpected err: %v", err) + require.True(t, len(newOnes) == 0, "expected none, got new=%d", len(newOnes)) + require.True(t, len(skipped) == 0, "expected none, got skipped=%d", len(skipped)) +} + +func TestGetGoalStatesToProcess_RespectsMaxTasksToFetch(t *testing.T) { + orig := validateSignatureFn + defer func() { validateSignatureFn = orig }() + validateSignatureFn = func(_ hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { + return true, nil + } + + gs := []hostgacommunicator.ImmediateExtensionGoalState{ + {Settings: []settings.SettingsCommon{ + mkSetting("RunCommand", 1, "A"), + mkSetting("RunCommand", 2, "B"), + mkSetting("RunCommand", 3, "C"), + }}, + } + + newOnes, skipped, err := getGoalStatesToProcess(gs, 2) + require.Nil(t, err, "unexpected err: %v", err) + require.Equal(t, 2, len(newOnes), "expected 2 new, got %d", len(newOnes)) + require.Equal(t, 1, len(skipped), "expected 1 skipped, got %d", len(skipped)) +} + +// ---- tests for processImmediateRunCommandGoalStates ---- + +func TestProcessImmediateRunCommandGoalStates_WhenAtCapacity_DoesNotFetch(t *testing.T) { + // Arrange + executingTasks = 0 + for i := int32(0); i < maxConcurrentTasks; i++ { + executingTasks.Increment() + } + defer func() { + // reset counter + for executingTasks.Get() > 0 { + executingTasks.Decrement() + } + }() + + origGet := getImmediateGoalStatesFn + defer func() { getImmediateGoalStatesFn = origGet }() + getImmediateGoalStatesFn = func(_ *log.Context, _ hostgacommunicator.IHostGACommunicator, _ string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, *vmextension.ErrorWithClarification) { + t.Fatalf("should not be called when at capacity") + return nil, "", nil + } + + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + var comm hostgacommunicator.HostGACommunicator // zero value ok for this test + + etag, err := processImmediateRunCommandGoalStates(ctx, comm, "etag-old") + require.Nil(t, err, "unexpected err: %v", err) + require.Equal(t, "etag-old", etag, "expected etag unchanged, got %q", etag) +} + +func TestProcessImmediateRunCommandGoalStates_WhenEtagUnchanged_NoWork(t *testing.T) { + // Arrange + for executingTasks.Get() > 0 { + executingTasks.Decrement() + } + + origGet := getImmediateGoalStatesFn + defer func() { getImmediateGoalStatesFn = origGet }() + getImmediateGoalStatesFn = func(_ *log.Context, _ hostgacommunicator.IHostGACommunicator, last string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, *vmextension.ErrorWithClarification) { + return nil, last, nil // unchanged + } + + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + var comm hostgacommunicator.HostGACommunicator + + etag, err := processImmediateRunCommandGoalStates(ctx, comm, "same") + require.Nil(t, err, "unexpected err: %v", err) + require.Equal(t, "same", etag, "expected same etag, got %q", etag) +} + +func TestProcessImmediateRunCommandGoalStates_GoalStateFailed(t *testing.T) { + // Make deterministic: run "goroutines" inline. + origSpawn := spawnFn + defer func() { spawnFn = origSpawn }() + spawnFn = func(f func()) { f() } + + // Make deterministic time. + origNow := nowFn + defer func() { nowFn = origNow }() + fixed := time.Date(2025, 12, 23, 10, 0, 0, 0, time.UTC) + nowFn = func() time.Time { return fixed } + + // Signature validation: true + origValidate := validateSignatureFn + defer func() { validateSignatureFn = origValidate }() + validateSignatureFn = func(_ hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { return true, nil } + + gs := []hostgacommunicator.ImmediateExtensionGoalState{ + {Settings: []settings.SettingsCommon{ + mkSetting("RunCommand", 1, "A"), + }}, + } + + origGet := getImmediateGoalStatesFn + defer func() { getImmediateGoalStatesFn = origGet }() + getImmediateGoalStatesFn = func(_ *log.Context, _ hostgacommunicator.IHostGACommunicator, _ string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, *vmextension.ErrorWithClarification) { + return gs, "etag-new", nil + } + + // HandleImmediateGoalState called exactly once (maxTasksToFetch=1), + // and we’ll return success (no final report for success). + handleCalls := 0 + origHandle := handleImmediateGoalStateFn + defer func() { handleImmediateGoalStateFn = origHandle }() + handleImmediateGoalStateFn = func(_ *log.Context, _ settings.SettingsCommon, _ *observer.Notifier) (int, *vmextension.ErrorWithClarification) { + handleCalls++ + return 0, vmextension.NewErrorWithClarificationPtr(constants.Hgap_InternalArgumentError, errors.New("the chipmunks do not see your argument")) + } + + // ReportFinalStatus called for the failed item + reportCalls := 0 + origReport := reportFinalStatusFn + defer func() { reportFinalStatusFn = origReport }() + reportFinalStatusFn = func(_ *log.Context, _ *observer.Notifier, _ types.GoalStateKey, statusType types.StatusType, instView *types.RunCommandInstanceView) error { + require.Equal(t, types.StatusError, statusType, "expected StatusError report, got %v", statusType) + require.Equal(t, constants.Hgap_InternalArgumentError, instView.ErrorClarificationValue, "expected %d error code, got %d", constants.Hgap_InternalArgumentError, instView.ExitCode) + reportCalls++ + return nil + } + + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + goalStateEventObserver.Initialize(ctx) + goalStateEventObserver.ReportImmediateStatusFn = func(s status.ImmediateTopLevelStatus) error { + return nil + } + + tmpDir := t.TempDir() + t.Setenv(constants.ExtensionPathEnvName, tmpDir) + + var comm hostgacommunicator.HostGACommunicator + + _, err := processImmediateRunCommandGoalStates(ctx, comm, "etag-old") + require.Nil(t, err, "unexpected err: %v", err) + require.Equal(t, 1, handleCalls, "expected 1 handled call, got %d", handleCalls) + require.Equal(t, 1, reportCalls, "expected 1 reports, got %d", reportCalls) +} + +func TestProcessImmediateRunCommandGoalStates_LaunchesAndReportsSkipped(t *testing.T) { + // Make deterministic: run "goroutines" inline. + origSpawn := spawnFn + defer func() { spawnFn = origSpawn }() + spawnFn = func(f func()) { f() } + + // Make deterministic time. + origNow := nowFn + defer func() { nowFn = origNow }() + fixed := time.Date(2025, 12, 23, 10, 0, 0, 0, time.UTC) + nowFn = func() time.Time { return fixed } + + // Signature validation: true + origValidate := validateSignatureFn + defer func() { validateSignatureFn = origValidate }() + validateSignatureFn = func(_ hostgacommunicator.ImmediateExtensionGoalState) (bool, error) { return true, nil } + + // Return 3 goal states; max tasks should be 5 in empty case, + // but we’ll artificially fill executingTasks to force maxTasksToFetch=1. + for executingTasks.Get() > 0 { + executingTasks.Decrement() + } + // executing=4 => maxTasksToFetch=1 + for i := 0; i < 4; i++ { + executingTasks.Increment() + } + defer func() { + for executingTasks.Get() > 0 { + executingTasks.Decrement() + } + }() + + gs := []hostgacommunicator.ImmediateExtensionGoalState{ + {Settings: []settings.SettingsCommon{ + mkSetting("RunCommand", 1, "A"), + mkSetting("RunCommand", 2, "B"), + mkSetting("RunCommand", 3, "C"), + }}, + } + + origGet := getImmediateGoalStatesFn + defer func() { getImmediateGoalStatesFn = origGet }() + getImmediateGoalStatesFn = func(_ *log.Context, _ hostgacommunicator.IHostGACommunicator, _ string) ([]hostgacommunicator.ImmediateExtensionGoalState, string, *vmextension.ErrorWithClarification) { + return gs, "etag-new", nil + } + + // HandleImmediateGoalState called exactly once (maxTasksToFetch=1), + // and we’ll return success (no final report for success). + handleCalls := 0 + origHandle := handleImmediateGoalStateFn + defer func() { handleImmediateGoalStateFn = origHandle }() + handleImmediateGoalStateFn = func(_ *log.Context, _ settings.SettingsCommon, _ *observer.Notifier) (int, *vmextension.ErrorWithClarification) { + handleCalls++ + return 0, nil + } + + // ReportFinalStatus called for skipped items (2 of them). + reportCalls := 0 + origReport := reportFinalStatusFn + defer func() { reportFinalStatusFn = origReport }() + reportFinalStatusFn = func(_ *log.Context, _ *observer.Notifier, _ types.GoalStateKey, statusType types.StatusType, instView *types.RunCommandInstanceView) error { + require.Equal(t, types.StatusSkipped, statusType, "expected StatusSkipped report, got %v", statusType) + require.Equal(t, constants.ImmediateRC_CommandSkipped, instView.ExitCode, "expected skipped exit code, got %d", instView.ExitCode) + reportCalls++ + return nil + } + + ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) + goalStateEventObserver.Initialize(ctx) + goalStateEventObserver.ReportImmediateStatusFn = func(s status.ImmediateTopLevelStatus) error { + return nil + } + + tmpDir := t.TempDir() + t.Setenv(constants.ExtensionPathEnvName, tmpDir) + + var comm hostgacommunicator.HostGACommunicator + + newEtag, err := processImmediateRunCommandGoalStates(ctx, comm, "etag-old") + require.Nil(t, err, "unexpected err: %v", err) + require.Equal(t, "etag-new", newEtag, "expected etag-new, got %q", newEtag) + require.Equal(t, 1, handleCalls, "expected 1 handled call, got %d", handleCalls) + require.Equal(t, 2, reportCalls, "expected 2 skipped reports, got %d", reportCalls) +} diff --git a/internal/instanceview/instanceview.go b/internal/instanceview/instanceview.go index 109c2d3..259d987 100755 --- a/internal/instanceview/instanceview.go +++ b/internal/instanceview/instanceview.go @@ -23,7 +23,11 @@ func ReportInstanceView(ctx *log.Context, hEnv types.HandlerEnvironment, metadat return err } - return c.Functions.ReportStatus(ctx, hEnv, metadata, t, c, msg) + if c.Functions.Pre == nil { + return c.Functions.ReportStatus(ctx, hEnv, metadata, t, c, msg) + } + slice := []int{instanceview.ExitCode} + return c.Functions.ReportStatus(ctx, hEnv, metadata, t, c, msg, slice...) } func SerializeInstanceView(instanceview *types.RunCommandInstanceView) (string, error) { diff --git a/internal/service/serviceinstall.go b/internal/service/serviceinstall.go index 1014e2f..603bd1a 100644 --- a/internal/service/serviceinstall.go +++ b/internal/service/serviceinstall.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/Azure/azure-extension-platform/pkg/extensionevents" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/servicehandler" "github.com/Azure/run-command-handler-linux/pkg/systemd" @@ -34,10 +35,17 @@ StandardError=append:%run_command_output_directory% WantedBy=multi-user.target` ) -func Register(ctx *log.Context, extensionEvents *extensionevents.ExtensionEventManager) error { +var ( + fnIsSystemDPresent = systemd.IsSystemDPresent + fnGetUnitManager = createUnitManager + fnChmod = os.Chmod +) + +func Register(ctx *log.Context, extensionEvents *extensionevents.ExtensionEventManager) *vmextension.ErrorWithClarification { if !isSystemdSupported(ctx) { - extensionEvents.LogErrorEvent("register", "Systemd not supported. Failed to register service") - return errors.New("Systemd not supported. Failed to register service") + errorMsg := "Systemd not supported. Failed to register servcice" + extensionEvents.LogErrorEvent("register", errorMsg) + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_Systemd_NotSupported, errors.New(errorMsg)) } targetVersion := os.Getenv(constants.ExtensionVersionEnvName) ctx.Log("message", "trying to register extension with version: "+targetVersion) @@ -48,14 +56,14 @@ func Register(ctx *log.Context, extensionEvents *extensionevents.ExtensionEventM isInstalled, err := IsInstalled(ctx) if err != nil { - return err + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotDetermineServiceInstalled, err) } // If the service is installed, check if it needs to be upgraded. if isInstalled { installedVersion, err := serviceHandler.GetInstalledVersion(ctx) if err != nil { - return err + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotDetermineInstalledVersion, err) } if installedVersion == targetVersion { @@ -68,21 +76,21 @@ func Register(ctx *log.Context, extensionEvents *extensionevents.ExtensionEventM ctx.Log("message", "Making immediate-run-command-handler executable") execDirectory := os.Getenv(constants.ExtensionPathEnvName) + "/bin/immediate-run-command-handler" - err = os.Chmod(execDirectory, 0744) + err = fnChmod(execDirectory, 0744) if err != nil { errMessage := fmt.Sprintf("Error while marking the immediate run command binary as executable: %v", err) extensionEvents.LogErrorEvent("register", errMessage) - return errors.Wrap(err, "error while marking the immediate run command binary as executable") + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotMarkBinaryAsExecutable, errors.Wrap(err, errMessage)) } - err = serviceHandler.Register(ctx, systemdUnitContent) - if err != nil { - return err + ewc := serviceHandler.Register(ctx, systemdUnitContent) + if ewc != nil { + return ewc } err = Start(ctx, extensionEvents) if err != nil { - return err + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotStartService, err) } extensionEvents.LogInformationalEvent("register", "Service registration complete") @@ -225,7 +233,7 @@ func IsInstalled(ctx *log.Context) (bool, error) { func getSystemdHandler(ctx *log.Context) *servicehandler.Handler { ctx.Log("message", "Getting service handler for "+systemdUnitName) config := servicehandler.NewConfiguration(systemdUnitName) - handler := servicehandler.NewHandler(systemd.NewUnitManager(), config, ctx) + handler := servicehandler.NewHandler(fnGetUnitManager(), config, ctx) return &handler } @@ -239,7 +247,7 @@ func generateServiceConfigurationContent(ctx *log.Context) string { func isSystemdSupported(ctx *log.Context) bool { ctx.Log("message", "Check if systemd is present on the system before applying next operation") - result := systemd.IsSystemDPresent() + result := fnIsSystemDPresent() if result { ctx.Log("message", "systemd was found on the system") @@ -249,3 +257,7 @@ func isSystemdSupported(ctx *log.Context) bool { return result } + +func createUnitManager() servicehandler.UnitManager { + return systemd.NewUnitManager() +} diff --git a/internal/service/serviceinstall_test.go b/internal/service/serviceinstall_test.go new file mode 100644 index 0000000..1394e04 --- /dev/null +++ b/internal/service/serviceinstall_test.go @@ -0,0 +1,567 @@ +package service + +import ( + "fmt" + "os" + "testing" + + "github.com/Azure/azure-extension-platform/pkg/extensionevents" + "github.com/Azure/azure-extension-platform/pkg/handlerenv" + "github.com/Azure/azure-extension-platform/pkg/logging" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" + "github.com/Azure/run-command-handler-linux/pkg/servicehandler" + "github.com/go-kit/kit/log" + "github.com/pkg/errors" + "github.com/stretchr/testify/require" +) + +type mockHandler struct { + IsInstalledRet bool + IsInstalledErr error + + GetInstalledVersionRet string + GetInstalledVersionErr error + + StartErr error + StopErr error + EnableErr error + DisableErr error + RemoveUnitConfigurationFileErr error + CreateUnitConfigurationFileErr error + DaemonReloadErr error + + IsActiveRet bool + IsActiveErr error + IsEnabledRet bool + IsEnabledErr error +} + +func (m *mockHandler) StartUnit(unitName string, ctx *log.Context) error { return m.StartErr } +func (m *mockHandler) StopUnit(unitName string, ctx *log.Context) error { return m.StopErr } +func (m *mockHandler) EnableUnit(unitName string, ctx *log.Context) error { return m.EnableErr } +func (m *mockHandler) DisableUnit(unitName string, ctx *log.Context) error { return m.DisableErr } +func (m *mockHandler) DaemonReload(unitName string, ctx *log.Context) error { return m.DaemonReloadErr } +func (m *mockHandler) IsUnitActive(unitName string, ctx *log.Context) error { return m.IsActiveErr } +func (m *mockHandler) IsUnitEnabled(unitName string, ctx *log.Context) (bool, error) { + return m.IsEnabledRet, m.IsEnabledErr +} +func (m *mockHandler) IsUnitInstalled(unitName string, ctx *log.Context) (bool, error) { + return m.IsInstalledRet, m.IsInstalledErr +} +func (m *mockHandler) RemoveUnitConfigurationFile(unitName string, ctx *log.Context) error { + return m.RemoveUnitConfigurationFileErr +} +func (m *mockHandler) CreateUnitConfigurationFile(unitName string, content []byte, ctx *log.Context) error { + return m.CreateUnitConfigurationFileErr +} +func (m *mockHandler) GetInstalledVersion(unitName string, ctx *log.Context) (string, error) { + return m.GetInstalledVersionRet, m.GetInstalledVersionErr +} + +func injectMocks(t *testing.T, sysd bool, handler servicehandler.UnitManager, chmodOverride func(name string, mode os.FileMode) error) func() { + setChmodOverride := chmodOverride + if setChmodOverride == nil { + setChmodOverride = func(name string, mode os.FileMode) error { + return nil + } + } + origIsSystemdPresent := fnIsSystemDPresent + origNewUnitManager := fnGetUnitManager + origChmod := fnChmod + + fnChmod = setChmodOverride + fnIsSystemDPresent = func() bool { return sysd } + fnGetUnitManager = func() servicehandler.UnitManager { + return handler + } + + return func() { + fnIsSystemDPresent = origIsSystemdPresent + fnGetUnitManager = origNewUnitManager + fnChmod = origChmod + } +} + +func TestRegister_SystemdUnsupported(t *testing.T) { + restore := injectMocks(t, false, &mockHandler{}, nil) + defer restore() + + tempDir, _ := os.MkdirTemp("", "SystemdUnsupported") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_Systemd_NotSupported, err) +} + +func TestRegister_IsInstalledError(t *testing.T) { + handler := &mockHandler{IsInstalledErr: errors.New("fail")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + os.Setenv(constants.ExtensionVersionEnvName, "1.0.0") + + tempDir, _ := os.MkdirTemp("", "IsInstalledError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_CouldNotDetermineServiceInstalled, err) +} + +func TestRegister_InstalledVersionCheckError(t *testing.T) { + handler := &mockHandler{ + IsInstalledRet: true, + GetInstalledVersionErr: errors.New("boom"), + } + restore := injectMocks(t, true, handler, nil) + defer restore() + + os.Setenv(constants.ExtensionVersionEnvName, "2.0") + tempDir, _ := os.MkdirTemp("", "InstalledVersionCheckError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_CouldNotDetermineInstalledVersion, err) +} + +func TestRegister_SameVersion_NoOp(t *testing.T) { + handler := &mockHandler{ + IsInstalledRet: true, + GetInstalledVersionRet: "3.1", + } + restore := injectMocks(t, true, handler, nil) + defer restore() + + os.Setenv(constants.ExtensionVersionEnvName, "3.1") + tempDir, _ := os.MkdirTemp("", "SameVersion") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + require.Nil(t, err) +} + +func TestRegister_ChmodFailure(t *testing.T) { + fnChmod = func(name string, mode os.FileMode) error { + return errors.New("the chipmunks are using this file") + } + handler := &mockHandler{} + restore := injectMocks(t, true, handler, fnChmod) + defer restore() + + tempDir, _ := os.MkdirTemp("", "ChModFailure") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_CouldNotMarkBinaryAsExecutable, err) +} + +func TestRegister_HandlerDaemonReloadFails(t *testing.T) { + handler := &mockHandler{DaemonReloadErr: errors.New("fail reg")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmpDir := t.TempDir() + os.Setenv(constants.ExtensionPathEnvName, tmpDir) + os.WriteFile(tmpDir+"/bin/immediate-run-command-handler", []byte("x"), 0755) + os.Setenv(constants.ExtensionVersionEnvName, "6.0") + + tempDir, _ := os.MkdirTemp("", "HandlerRegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_ErrorReloadingDaemonWorker, err) +} + +func TestRegister_HandlerRemoveUnitConfigurationFails(t *testing.T) { + handler := &mockHandler{RemoveUnitConfigurationFileErr: errors.New("fail unit config")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmpDir := t.TempDir() + os.Setenv(constants.ExtensionPathEnvName, tmpDir) + os.WriteFile(tmpDir+"/bin/immediate-run-command-handler", []byte("x"), 0755) + os.Setenv(constants.ExtensionVersionEnvName, "6.0") + + tempDir, _ := os.MkdirTemp("", "HandlerRegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_CouldNotRemoveOldUnitConfigFile, err) +} + +func TestRegister_HandlerCreateUnitConfigurationFails(t *testing.T) { + handler := &mockHandler{CreateUnitConfigurationFileErr: errors.New("fail unit config")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmpDir := t.TempDir() + os.Setenv(constants.ExtensionPathEnvName, tmpDir) + os.WriteFile(tmpDir+"/bin/immediate-run-command-handler", []byte("x"), 0755) + os.Setenv(constants.ExtensionVersionEnvName, "6.0") + + tempDir, _ := os.MkdirTemp("", "HandlerRegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_ErrorCreatingUnitConfig, err) +} + +func TestRegister_HandlerEnableUnitFails(t *testing.T) { + handler := &mockHandler{EnableErr: errors.New("fail enable")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmpDir := t.TempDir() + os.Setenv(constants.ExtensionPathEnvName, tmpDir) + os.WriteFile(tmpDir+"/bin/immediate-run-command-handler", []byte("x"), 0755) + os.Setenv(constants.ExtensionVersionEnvName, "6.0") + + tempDir, _ := os.MkdirTemp("", "HandlerRegisterFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_ErrorEnablingUnit, err) +} + +func TestRegister_StartFails(t *testing.T) { + handler := &mockHandler{StartErr: errors.New("cannot start")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmp := t.TempDir() + os.MkdirAll(tmp+"/bin", 0755) + os.WriteFile(tmp+"/bin/immediate-run-command-handler", []byte("x"), 0755) + + os.Setenv(constants.ExtensionPathEnvName, tmp) + os.Setenv(constants.ExtensionVersionEnvName, "7.0") + + tempDir, _ := os.MkdirTemp("", "StartFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + VerifyErrorClarification(t, constants.Immediate_CouldNotStartService, err) +} + +func TestRegister_Success(t *testing.T) { + handler := &mockHandler{} + restore := injectMocks(t, true, handler, nil) + defer restore() + + tmp := t.TempDir() + os.MkdirAll(tmp+"/bin", 0755) + os.WriteFile(tmp+"/bin/immediate-run-command-handler", []byte("run"), 0755) + + os.Setenv(constants.ExtensionPathEnvName, tmp) + os.Setenv(constants.ExtensionVersionEnvName, "1.9") + + tempDir, _ := os.MkdirTemp("", "TestRegister") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + ctx := log.NewContext(log.NewNopLogger()) + + err := Register(ctx, evt) + require.Nil(t, err) +} + +func TestDeRegister_Success(t *testing.T) { + handler := &mockHandler{} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "TestDeRegister") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.NoError(t, DeRegister(ctx, evt)) +} + +func TestDeRegister_SystemdUnsupported(t *testing.T) { + restore := injectMocks(t, false, &mockHandler{}, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "SystemdUnsupported") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.NoError(t, DeRegister(ctx, evt)) // No-op +} + +func TestEnable_Error(t *testing.T) { + handler := &mockHandler{EnableErr: errors.New("fail")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "EnableError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.Error(t, Enable(ctx, evt)) +} + +func TestEnable_Success(t *testing.T) { + handler := &mockHandler{} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "EnableSuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.NoError(t, Enable(ctx, evt)) +} + +func TestDisable_Success(t *testing.T) { + handler := &mockHandler{} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "DisableSuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.NoError(t, Disable(ctx, evt)) +} + +func TestDisable_StopFails(t *testing.T) { + handler := &mockHandler{StopErr: errors.New("stop err")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "StopFails") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.Error(t, Disable(ctx, evt)) +} + +func TestStop_Success(t *testing.T) { + handler := &mockHandler{} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "StopSuccess") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.NoError(t, Stop(ctx, evt)) +} + +func TestStop_Error(t *testing.T) { + handler := &mockHandler{StopErr: fmt.Errorf("fail stop")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + tempDir, _ := os.MkdirTemp("", "StopError") + defer os.RemoveAll(tempDir) + handlerEnvironment := handlerenv.HandlerEnvironment{ + EventsFolder: tempDir, + } + + extensionLogger := logging.New(nil) + evt := extensionevents.New(extensionLogger, &handlerEnvironment) + + require.Error(t, Stop(ctx, evt)) +} + +func TestIsActive_Success(t *testing.T) { + handler := &mockHandler{IsActiveRet: true} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + ok, err := IsActive(ctx) + require.NoError(t, err) + require.True(t, ok) +} + +func TestIsActive_Error(t *testing.T) { + handler := &mockHandler{IsActiveErr: errors.New("fail")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + _, err := IsActive(ctx) + require.Error(t, err) +} + +func TestIsInstalled_NoSystemd(t *testing.T) { + restore := injectMocks(t, false, &mockHandler{}, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + ok, err := IsInstalled(ctx) + require.NoError(t, err) + require.False(t, ok) +} + +func TestIsInstalled_Success(t *testing.T) { + handler := &mockHandler{IsInstalledRet: true} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + ok, err := IsInstalled(ctx) + require.NoError(t, err) + require.True(t, ok) +} + +func TestIsInstalled_Error(t *testing.T) { + handler := &mockHandler{IsInstalledErr: errors.New("fail install")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + _, err := IsInstalled(ctx) + require.Error(t, err) +} + +func TestIsEnabled_Success(t *testing.T) { + handler := &mockHandler{IsEnabledRet: true} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + ok, err := IsEnabled(ctx) + require.NoError(t, err) + require.True(t, ok) +} + +func TestIsEnabled_Error(t *testing.T) { + handler := &mockHandler{IsEnabledErr: errors.New("fail enabled")} + restore := injectMocks(t, true, handler, nil) + defer restore() + + ctx := log.NewContext(log.NewNopLogger()) + + _, err := IsEnabled(ctx) + require.Error(t, err) +} + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/internal/status/immediatestatus.go b/internal/status/immediatestatus.go index 51b0944..86d4174 100644 --- a/internal/status/immediatestatus.go +++ b/internal/status/immediatestatus.go @@ -3,6 +3,7 @@ package status import ( "encoding/json" "fmt" + "reflect" "slices" "sync" @@ -43,16 +44,23 @@ type StatusObserver struct { // Reporter is the status Reporter Reporter statusreporter.IGuestInformationServiceClient + + ReportImmediateStatusFn func(s ImmediateTopLevelStatus) error } func (o *StatusObserver) Initialize(ctx *log.Context) { o.goalStateEventMap = sync.Map{} o.ctx = ctx o.Reporter = statusreporter.NewGuestInformationServiceClient(hostgacommunicator.WireServerFallbackAddress) + + o.ReportImmediateStatusFn = func(s ImmediateTopLevelStatus) error { + return o.reportImmediateStatus(s) + } } func (o *StatusObserver) OnDemandNotify() error { - return o.reportImmediateStatus(o.getImmediateTopLevelStatusToReport()) + status := o.getImmediateTopLevelStatusToReport() + return o.ReportImmediateStatusFn(status) } func (o *StatusObserver) OnNotify(status types.StatusEventArgs) error { @@ -60,7 +68,9 @@ func (o *StatusObserver) OnNotify(status types.StatusEventArgs) error { o.goalStateEventMap.Store(status.StatusKey, status.TopLevelStatus) return o.OnDemandNotify() } - +func IsEmptyStatusItem(statusItem1 types.StatusItem) bool { + return reflect.DeepEqual(statusItem1, types.StatusItem{}) +} func (o *StatusObserver) getImmediateTopLevelStatusToReport() ImmediateTopLevelStatus { latestStatusToReport := []ImmediateStatus{} goalStateKeysToCheckToRemove := []types.GoalStateKey{} @@ -71,7 +81,7 @@ func (o *StatusObserver) getImmediateTopLevelStatusToReport() ImmediateTopLevelS // Only report the latest active status for each goal state goalStateKey := key.(types.GoalStateKey) if goalStateKey.RuntimeSettingsState != "disabled" { - if value.(types.StatusItem) != (types.StatusItem{}) { + if !IsEmptyStatusItem(value.(types.StatusItem)) { o.ctx.Log("message", fmt.Sprintf("Goal state %v is not empty. Processing it.", goalStateKey)) statusItem := value.(types.StatusItem) immediateStatus := ImmediateStatus{ @@ -148,11 +158,12 @@ func (o *StatusObserver) reportImmediateStatus(immediateStatus ImmediateTopLevel o.ctx.Log("message", "create request to upload status to: "+o.Reporter.GetPutStatusUri()) response, err := o.Reporter.ReportStatus(o.ctx, string(rootStatusJson)) - o.ctx.Log("message", fmt.Sprintf("Status received from request to %v: %v", response.Request.URL, response.Status)) if err != nil { return errors.Wrap(err, "failed to report status to HGAP") } + o.ctx.Log("message", fmt.Sprintf("Status received from request to %v: %v", response.Request.URL, response.Status)) + if response.StatusCode != 200 { return errors.New("failed to report status with error code " + response.Status) } diff --git a/internal/status/status.go b/internal/status/status.go index 6924265..7011e7e 100755 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -22,13 +22,21 @@ var immediateGSInTerminalStatusLock = sync.Mutex{} // If an error occurs reporting the status, it will be logged and returned. // // This function is used by default for reporting status to the local file system unless a different method is specified. -func ReportStatusToLocalFile(ctx *log.Context, hEnv types.HandlerEnvironment, metadata types.RCMetadata, statusType types.StatusType, c types.Cmd, msg string) error { +func ReportStatusToLocalFile(ctx *log.Context, hEnv types.HandlerEnvironment, metadata types.RCMetadata, statusType types.StatusType, c types.Cmd, msg string, exitCode ...int) error { if !c.ShouldReportStatus { ctx.Log("status", "not reported for operation (by design)") return nil } rootStatusJson, err := getRootStatusJson(ctx, statusType, c, msg, true, metadata.ExtName) + if c.Functions.Pre != nil { + errorCode := 0 + if len(exitCode) > 0 { + errorCode = exitCode[0] + } + rootStatusJson, err = getRootStatusJsonWithErrorClarification(ctx, statusType, c, msg, true, metadata.ExtName, errorCode) + } + if err != nil { return errors.Wrap(err, "failed to get json for status report") } @@ -79,6 +87,10 @@ func SaveGoalStatesInTerminalStatus(ctx *log.Context, newStatusInTerminalState [ newExtensionDirectory := os.Getenv(constants.ExtensionPathEnvName) immediateStatusFolder := filepath.Join(newExtensionDirectory, constants.ImmediateStatusFileDirectory) + if err := os.MkdirAll(immediateStatusFolder, 0755); err != nil { + return fmt.Errorf("status: failed to create directory %q: %v", immediateStatusFolder, err) + } + ctx.Log("message", "saving goal states in terminal state to file") statusFile := filepath.Join(immediateStatusFolder, constants.ImmediateGoalStatesInTerminalStatusFileName) tempStatusFile := statusFile + ".tmp" @@ -190,6 +202,7 @@ func RemoveDisabledAndUpdatedGoalStatesInLocalStatusFile(ctx *log.Context, goalS func getRootStatusJson(ctx *log.Context, statusType types.StatusType, c types.Cmd, msg string, indent bool, extName string) ([]byte, error) { ctx.Log("message", "creating json to report status") + statusReport := types.NewStatusReport(statusType, c.Name, msg, extName) b, err := MarshalStatusReportIntoJson(statusReport, indent) @@ -200,6 +213,18 @@ func getRootStatusJson(ctx *log.Context, statusType types.StatusType, c types.Cm return b, nil } +func getRootStatusJsonWithErrorClarification(ctx *log.Context, statusType types.StatusType, c types.Cmd, msg string, indent bool, extName string, errorcode int) ([]byte, error) { + ctx.Log("message", "creating json to report status") + statusReport := types.NewStatusReportWithErrorClarification(statusType, c.Name, msg, extName, errorcode) + + b, err := MarshalStatusReportIntoJson(statusReport, indent) + if err != nil { + return nil, errors.Wrap(err, "failed to marshal status report into json") + } + + return b, nil +} + // getSingleStatusItem returns a single status item for the given status type, command, and message. // This is useful when only a single status item is needed for an immediate status report. func GetSingleStatusItem(ctx *log.Context, statusType types.StatusType, c types.Cmd, msg string, extName string) (types.StatusItem, error) { diff --git a/internal/status/status_test.go b/internal/status/status_test.go index 55a46de..b1c568d 100644 --- a/internal/status/status_test.go +++ b/internal/status/status_test.go @@ -21,6 +21,16 @@ func Test_reportStatus_fails(t *testing.T) { require.Contains(t, err.Error(), "failed to save handler status") } +func Test_reportStatusWithClarification_fails(t *testing.T) { + fakeEnv := types.HandlerEnvironment{} + fakeEnv.HandlerEnvironment.StatusFolder = "/non-existing/dir/" + + metadata := types.NewRCMetadata("", 1, constants.DownloadFolder, constants.DataDir) + err := ReportStatusToLocalFile(log.NewContext(log.NewNopLogger()), fakeEnv, metadata, types.StatusSuccess, types.CmdEnableTemplate, "", 0) + require.NotNil(t, err) + require.Contains(t, err.Error(), "failed to save handler status") +} + func Test_reportStatus_fileExists(t *testing.T) { tmpDir, err := os.MkdirTemp("", "") require.Nil(t, err) @@ -39,6 +49,24 @@ func Test_reportStatus_fileExists(t *testing.T) { require.NotEqual(t, 0, len(b), ".status file not empty") } +func Test_reportStatusWithClarification_fileExists(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "") + require.Nil(t, err) + defer os.RemoveAll(tmpDir) + + extName := "first" + fakeEnv := types.HandlerEnvironment{} + fakeEnv.HandlerEnvironment.StatusFolder = tmpDir + + metadata := types.NewRCMetadata(extName, 1, constants.DownloadFolder, constants.DataDir) + require.Nil(t, ReportStatusToLocalFile(log.NewContext(log.NewNopLogger()), fakeEnv, metadata, types.StatusError, types.CmdEnableTemplate, "FOO ERROR", 0)) + + path := filepath.Join(tmpDir, "first.1.status") + b, err := os.ReadFile(path) + require.Nil(t, err, ".status file exists") + require.NotEqual(t, 0, len(b), ".status file not empty") +} + func Test_reportStatus_checksIfShouldBeReported(t *testing.T) { for _, c := range types.CmdTemplates { tmpDir, err := os.MkdirTemp("", "status-"+c.Name) @@ -66,6 +94,32 @@ func Test_reportStatus_checksIfShouldBeReported(t *testing.T) { } } +func Test_reportStatusWithClarification_checksIfShouldBeReported(t *testing.T) { + for _, c := range types.CmdTemplates { + tmpDir, err := os.MkdirTemp("", "status-"+c.Name) + require.Nil(t, err) + defer os.RemoveAll(tmpDir) + + extName := "first" + fakeEnv := types.HandlerEnvironment{} + fakeEnv.HandlerEnvironment.StatusFolder = tmpDir + metadata := types.NewRCMetadata(extName, 2, constants.DownloadFolder, constants.DataDir) + require.Nil(t, ReportStatusToLocalFile(log.NewContext(log.NewNopLogger()), fakeEnv, metadata, types.StatusSuccess, c, "", 0)) + + fp := filepath.Join(tmpDir, "first.2.status") + _, err = os.Stat(fp) // check if the .status file is there + if c.ShouldReportStatus && err != nil { + t.Fatalf("cmd=%q should have reported status file=%q err=%v", c.Name, fp, err) + } + if !c.ShouldReportStatus { + if err == nil { + t.Fatalf("cmd=%q should not have reported status file. file=%q", c.Name, fp) + } else if !os.IsNotExist(err) { + t.Fatalf("cmd=%q some other error occurred. file=%q err=%q", c.Name, fp, err) + } + } + } +} func Test_getSingleStatusItem(t *testing.T) { ctx := log.NewContext(log.NewSyncLogger(log.NewLogfmtLogger(os.Stdout))).With("time", log.DefaultTimestamp) msgToReport := "Final message to report" diff --git a/internal/types/commands.go b/internal/types/commands.go index 617a81b..82f580a 100755 --- a/internal/types/commands.go +++ b/internal/types/commands.go @@ -5,7 +5,7 @@ import ( ) type cmdFunc func(ctx *log.Context, hEnv HandlerEnvironment, report *RunCommandInstanceView, metadata RCMetadata, c Cmd) (stdout string, stderr string, err error, exitCode int) -type reportStatusFunc func(ctx *log.Context, hEnv HandlerEnvironment, metadata RCMetadata, statusType StatusType, c Cmd, msg string) error +type reportStatusFunc func(ctx *log.Context, hEnv HandlerEnvironment, metadata RCMetadata, statusType StatusType, c Cmd, msg string, exitcode ...int) error type preFunc func(ctx *log.Context, hEnv HandlerEnvironment, metadata RCMetadata, c Cmd) error type cleanupFunc func(ctx *log.Context, metadata RCMetadata, h HandlerEnvironment, runAsUser string) diff --git a/internal/types/handlerenvironment.go b/internal/types/handlerenvironment.go index 19b9736..62f58a0 100644 --- a/internal/types/handlerenvironment.go +++ b/internal/types/handlerenvironment.go @@ -5,16 +5,18 @@ package types type HandlerEnvironment struct { Version float64 `json:"version"` Name string `json:"name"` - HandlerEnvironment struct { - HeartbeatFile string `json:"heartbeatFile"` - StatusFolder string `json:"statusFolder"` - ConfigFolder string `json:"configFolder"` - LogFolder string `json:"logFolder"` - EventsFolder string `json:"eventsFolder"` - EventsFolderPreview string `json:"eventsFolder_preview"` - DeploymentID string `json:"deploymentid"` - RoleName string `json:"rolename"` - Instance string `json:"instance"` - HostResolverAddress string `json:"hostResolverAddress"` - } + HandlerEnvironment HandlerEnvironmentDetails +} + +type HandlerEnvironmentDetails struct { + HeartbeatFile string `json:"heartbeatFile"` + StatusFolder string `json:"statusFolder"` + ConfigFolder string `json:"configFolder"` + LogFolder string `json:"logFolder"` + EventsFolder string `json:"eventsFolder"` + EventsFolderPreview string `json:"eventsFolderPreview"` + DeploymentID string `json:"deploymentid"` + RoleName string `json:"rolename"` + Instance string `json:"instance"` + HostResolverAddress string `json:"hostResolverAddress"` } diff --git a/internal/types/instanceview.go b/internal/types/instanceview.go index 31774fb..7aa8336 100644 --- a/internal/types/instanceview.go +++ b/internal/types/instanceview.go @@ -30,13 +30,14 @@ const ( // RunCommandInstanceView reports script execution status type RunCommandInstanceView struct { - ExecutionState ExecutionState `json:"executionState"` - ExecutionMessage string `json:"executionMessage"` - Output string `json:"output"` - Error string `json:"error"` - ExitCode int `json:"exitCode"` - StartTime string `json:"startTime"` - EndTime string `json:"endTime"` + ExecutionState ExecutionState `json:"executionState"` + ExecutionMessage string `json:"executionMessage"` + Output string `json:"output"` + Error string `json:"error"` + ExitCode int `json:"exitCode"` + StartTime string `json:"startTime"` + EndTime string `json:"endTime"` + ErrorClarificationValue int `json:"errorClarificationValue,omitempty"` } func (instanceView RunCommandInstanceView) Marshal() ([]byte, error) { diff --git a/internal/types/status.go b/internal/types/status.go index 8745502..1661b3b 100755 --- a/internal/types/status.go +++ b/internal/types/status.go @@ -6,6 +6,7 @@ import "time" type StatusReport []StatusItem func NewStatusReport(statusType StatusType, operation string, message string, extName string) StatusReport { + return []StatusItem{ { Version: 1, // this is the protocol version do not change unless you are sure @@ -22,6 +23,37 @@ func NewStatusReport(statusType StatusType, operation string, message string, ex } } +func NewStatusReportWithErrorClarification(statusType StatusType, operation string, message string, extName string, errorcode int) StatusReport { + errorClarificationName := "ErrorClarification" + + var subStatuses []subStatus + + // Add subStatus only if errorcode is non-zero + if errorcode != 0 { + subStatuses = append(subStatuses, subStatus{ + Name: errorClarificationName, + Code: errorcode, + Status: statusType, + }) + } + + return []StatusItem{ + { + Version: 1, // this is the protocol version do not change unless you are sure + TimestampUTC: time.Now().UTC().Format(time.RFC3339), + Status: Status{ + Name: extName, + Operation: operation, + Status: statusType, + FormattedMessage: FormattedMessage{ + Lang: "en", + Message: message}, + SubStatus: subStatuses, + }, + }, + } +} + // StatusItem is used to serialize an individual part of the status read by the server type StatusItem struct { Version int `json:"version"` @@ -52,6 +84,7 @@ type Status struct { Operation string `json:"operation"` Status StatusType `json:"status"` FormattedMessage FormattedMessage `json:"formattedMessage"` + SubStatus []subStatus `json:"substatus"` // optional substatus, can be nil } // FormattedMessage is a struct used for serializing status @@ -59,3 +92,17 @@ type FormattedMessage struct { Lang string `json:"lang"` Message string `json:"message"` } + +// substatus used for serialization +// It contains neccesary info that is used in CRP for error clarification +type subStatus struct { + // Name is the name of the substatus + // Should be set as "ErroClarificationName" + Name string `json:"name"` + // Code is the code of the substatus + // Number code that is used in CRP for error clarification in conjunction with the errorclassification file + Code int `json:"code"` + // Status is the status of the substatus + // Status of the run command operation + Status StatusType `json:"status"` +} diff --git a/pkg/download/blob.go b/pkg/download/blob.go index efb3b5a..96a5c8a 100755 --- a/pkg/download/blob.go +++ b/pkg/download/blob.go @@ -10,7 +10,9 @@ import ( "strings" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/azure-sdk-for-go/storage" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/blobutil" "github.com/google/uuid" "github.com/pkg/errors" @@ -28,9 +30,9 @@ type blobDownload struct { } func (b blobDownload) GetRequest() (*http.Request, error) { - url, err := b.getURL() - if err != nil { - return nil, err + url, ewc := b.getURL() + if ewc != nil { + return nil, ewc } req, err := http.NewRequest("GET", url, nil) if req != nil { @@ -41,11 +43,11 @@ func (b blobDownload) GetRequest() (*http.Request, error) { // getURL returns publicly downloadable URL of the Azure Blob // by generating a URL with a temporary Shared Access Signature. -func (b blobDownload) getURL() (string, error) { +func (b blobDownload) getURL() (string, *vmextension.ErrorWithClarification) { client, err := storage.NewClient(b.accountName, b.accountKey, b.blob.StorageBase, storage.DefaultAPIVersion, true) if err != nil { - return "", errors.Wrap(err, "failed to initialize azure storage client") + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_StorageClientInitialization, errors.Wrap(err, "failed to initialize azure storage client")) } // get read-only @@ -61,7 +63,7 @@ func (b blobDownload) getURL() (string, error) { sasURL, err := blob.GetSASURI(options) if err != nil { - return "", errors.Wrap(err, "failed to generate SAS key for blob") + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CannotGenerateSasKey, errors.Wrap(err, "failed to generate SAS key for blob")) } return sasURL, nil } @@ -73,25 +75,25 @@ func NewBlobDownload(accountName, accountKey string, blob blobutil.AzureBlobRef) // GetSASBlob download a blob with specified uri and sas authorization and saves it to the target directory // Returns the filePath where the blob was downloaded -func GetSASBlob(blobURI, blobSas, targetDir string) (string, error) { +func GetSASBlob(blobURI, blobSas, targetDir string) (string, *vmextension.ErrorWithClarification) { blobFullURL := blobURI + blobSas loggableBlobUri := GetUriForLogging(blobURI) resp, err := http.Get(blobFullURL) if err != nil { - return "", errors.Wrapf(err, "Failed to download file: %q", loggableBlobUri) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_GenericError, errors.Wrapf(err, "Failed to download file: %q", loggableBlobUri)) } defer resp.Body.Close() // Ensure the response body is closed after we're done // Check if the HTTP status code indicates success (e.g., 200 OK) if resp.StatusCode != http.StatusOK { - return "", errors.Wrapf(err, "Failed to download file: %q, Http status code: %s", loggableBlobUri, resp.Status) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_FailedStatusCode, errors.Wrapf(err, "Failed to download file: %q, Http status code: %s", loggableBlobUri, resp.Status)) } blobParsedurl, err := url.Parse(blobURI) if err != nil { - return "", errors.Wrapf(err, "unable to parse URL: %q", loggableBlobUri) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CannotParseUrl, errors.Wrapf(err, "unable to parse URL: %q", loggableBlobUri)) } // Extract container name from Path of the url https:/// For ex. Path = "containerName/dir1/dir2/file.sh" trimmedPath := strings.Trim(blobParsedurl.Path, "/") @@ -101,7 +103,7 @@ func GetSASBlob(blobURI, blobSas, targetDir string) (string, error) { // Extract the blob path after container name fileName, blobPathError := getBlobPathAfterContainerName(blobURI, containerName) if fileName == "" || blobPathError != nil { - return "", errors.Wrapf(blobPathError, "Failed to extract blob path name from URL: %q", loggableBlobUri) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_CannotExtractFileNameFromUrl, errors.Wrapf(blobPathError, "Failed to extract blob path name from URL: %q", loggableBlobUri)) } // Create the local file @@ -109,7 +111,7 @@ func GetSASBlob(blobURI, blobSas, targetDir string) (string, error) { const mode = 0500 // scripts should have execute permissions outFile, err := os.OpenFile(scriptFilePath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode) if err != nil { - return "", errors.Wrapf(err, "Failed to open file '%s' for writing: ", scriptFilePath) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_UnableToWriteFile, errors.Wrapf(err, "Failed to open file '%s' for writing: ", scriptFilePath)) } defer outFile.Close() // Ensure the file is closed after we're done @@ -118,34 +120,34 @@ func GetSASBlob(blobURI, blobSas, targetDir string) (string, error) { // loading the entire file into memory. _, err = io.Copy(outFile, resp.Body) if err != nil { - return "", errors.Wrapf(err, "Failed to copy data to file '%s'", scriptFilePath) + return "", vmextension.NewErrorWithClarificationPtr(constants.FileDownload_UnableToWriteFile, errors.Wrapf(err, "Failed to copy data to file '%s'", scriptFilePath)) } return scriptFilePath, nil } // CreateOrReplaceAppendBlob creates a reference to an append blob. If blob exists - it gets deleted first. -func CreateOrReplaceAppendBlob(blobURI, blobSas string) (*storage.Blob, error) { +func CreateOrReplaceAppendBlob(blobURI, blobSas string) (*storage.Blob, *vmextension.ErrorWithClarification) { bloburl, err := url.Parse(blobURI + blobSas) if err != nil { - return nil, err + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_InvalidUri, err) } containerRef, err := storage.GetContainerReferenceFromSASURI(*bloburl) if err != nil { - return nil, err + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_InvalidUri, err) } fileName, blobPathError := getBlobPathAfterContainerName(blobURI, containerRef.Name) if fileName == "" { - return nil, errors.Wrapf(blobPathError, "cannot extract blob path name from URL: %q", GetUriForLogging(blobURI)) + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_InvalidUri, errors.Wrapf(blobPathError, "cannot extract blob path name from URL: %q", GetUriForLogging(blobURI))) } blobref := containerRef.GetBlobReference(fileName) err = blobref.PutAppendBlob(nil) // Create the append blob if err != nil { - return nil, err + return nil, vmextension.NewErrorWithClarificationPtr(constants.AppendBlobCreation_Other, err) } return blobref, nil diff --git a/pkg/download/blob_test.go b/pkg/download/blob_test.go index d2232e7..e595545 100644 --- a/pkg/download/blob_test.go +++ b/pkg/download/blob_test.go @@ -1,15 +1,21 @@ package download import ( + "errors" "fmt" + "io" "io/ioutil" "math/rand" "net/http" + "net/http/httptest" "os" + "path/filepath" "testing" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/azure-sdk-for-go/storage" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/blobutil" "github.com/go-kit/kit/log" "github.com/google/uuid" @@ -30,6 +36,9 @@ func Test_blobDownload_validateInputs(t *testing.T) { errorMessage := err.Error() require.Contains(t, errorMessage, "failed to initialize azure storage client") require.Contains(t, errorMessage, "azure: account name is not valid") + var ewc *vmextension.ErrorWithClarification + require.True(t, errors.As(err, &ewc), "Error is not of type ErrorWithClarification") + VerifyErrorClarification(t, constants.FileDownload_StorageClientInitialization, ewc) _, err = NewBlobDownload("account", "", blobutil.AzureBlobRef{}).GetRequest() require.NotNil(t, err) @@ -71,6 +80,24 @@ func Test_blobDownload_getURL(t *testing.T) { } } +func Test_blobDownload_getURL_cannotGenerateSas(t *testing.T) { + type sas interface { + getURL() (string, error) + } + + d := NewBlobDownload("account", "Zm9vCg==", blobutil.AzureBlobRef{ + StorageBase: "!@#$%^&*()(_+)", + Container: "", + Blob: "blob.txt", + }) + + v, ok := d.(blobDownload) + require.True(t, ok) + + _, err := v.getURL() + VerifyErrorClarification(t, constants.FileDownload_CannotGenerateSasKey, err) +} + func Test_blobDownload_fails_badCreds(t *testing.T) { d := NewBlobDownload("example", "Zm9vCg==", blobutil.AzureBlobRef{ StorageBase: storage.DefaultBaseURL, @@ -95,6 +122,7 @@ func Test_blobDownload_fails_badCreds(t *testing.T) { require.Contains(t, err.Error(), "Please verify the machine has network connectivity") require.Contains(t, err.Error(), "403") require.Equal(t, status, http.StatusForbidden) + VerifyErrorClarification(t, constants.FileDownload_NetworkingError, err) } // Tests that a common error message will be uniquely formatted @@ -123,6 +151,85 @@ func Test_blobDownload_fails_badRequest(t *testing.T) { require.Contains(t, err.Error(), "parts of the request were incorrectly formatted, missing, and/or invalid") require.Contains(t, err.Error(), "400") require.Equal(t, status, http.StatusBadRequest) + VerifyErrorClarification(t, constants.FileDownload_BadRequest, err) +} + +func TestGetSASBlob_StatusNotOK_ReturnsFailedStatusCode(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + tmp := t.TempDir() + blobURI := srv.URL + "/container/file.txt" + blobSas := "?sig=dummy" + + _, err := GetSASBlob(blobURI, blobSas, tmp) + VerifyErrorClarification(t, constants.FileDownload_FailedStatusCode, err) +} + +func TestGetSASBlob_CannotExtractFileName_ReturnsCannotExtract(t *testing.T) { + // Return 200 so we get past the status-code check, then fail on filename extraction + // by giving a URL that has ONLY the container and no blob suffix. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "x") + })) + defer srv.Close() + + tmp := t.TempDir() + blobURI := srv.URL + "/container" // <- no "/file" + blobSas := "?sig=dummy" + + _, err := GetSASBlob(blobURI, blobSas, tmp) + VerifyErrorClarification(t, constants.FileDownload_CannotExtractFileNameFromUrl, err) +} + +func TestGetSASBlob_TargetDirMissing_ReturnsUnableToWriteFile(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "content") + })) + defer srv.Close() + + // Intentionally DO NOT create this directory. + tmp := t.TempDir() + missingDir := filepath.Join(tmp, "does-not-exist") + + blobURI := srv.URL + "/container/file.txt" + blobSas := "?sig=dummy" + + _, err := GetSASBlob(blobURI, blobSas, missingDir) + VerifyErrorClarification(t, constants.FileDownload_UnableToWriteFile, err) +} + +func TestGetSASBlob_HttpGetFails_ReturnsGenericError(t *testing.T) { + // invalid URL => http.Get fails + tmp := t.TempDir() + blobURI := "http://[::1" // invalid (unclosed bracket) + blobSas := "?sig=dummy" + + _, err := GetSASBlob(blobURI, blobSas, tmp) + VerifyErrorClarification(t, constants.FileDownload_GenericError, err) +} + +func TestCreateOrReplaceAppendBlob_InvalidUri_ReturnsInvalidUri(t *testing.T) { + _, err := CreateOrReplaceAppendBlob("http://[::1", "?sig=x") + VerifyErrorClarification(t, constants.AppendBlobCreation_InvalidUri, err) +} + +func TestCreateOrReplaceAppendBlob_MissingBlobName_ReturnsInvalidUri(t *testing.T) { + // Server never called; extraction should fail because there's no blob path after container. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("server should not have been called") + })) + defer srv.Close() + + blobURI := srv.URL + "/container" // no blob name + blobSas := "?sig=dummy" + + _, err := CreateOrReplaceAppendBlob(blobURI, blobSas) + VerifyErrorClarification(t, constants.AppendBlobCreation_InvalidUri, err) } func Test_blobDownload_fails_urlNotFound(t *testing.T) { @@ -211,10 +318,10 @@ func Test_blobAppend_actualBlob(t *testing.T) { t.Skipf("Skipping: AZURE_STORAGE_BLOB or SASTOKEN not specified to run this test") } - blobref, err := CreateOrReplaceAppendBlob(blobURI, sasToken) - require.Nil(t, err) + blobref, ewc := CreateOrReplaceAppendBlob(blobURI, sasToken) + require.Nil(t, ewc) - err = blobref.AppendBlock([]byte("First line\n"), nil) + err := blobref.AppendBlock([]byte("First line\n"), nil) err = blobref.AppendBlock([]byte("Second line\n"), nil) err = blobref.AppendBlock([]byte("Third line\n"), nil) require.Nil(t, err) @@ -236,3 +343,8 @@ func (b badRequestBlobDownload) GetRequest() (*http.Request, error) { } return req, error } + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/pkg/download/blobwithmsitoken_test.go b/pkg/download/blobwithmsitoken_test.go index f223b70..8bee469 100644 --- a/pkg/download/blobwithmsitoken_test.go +++ b/pkg/download/blobwithmsitoken_test.go @@ -3,6 +3,7 @@ package download import ( "encoding/json" "io/ioutil" + // "net/http" "testing" @@ -46,8 +47,8 @@ func Test_realDownloadBlobWithMsiToken(t *testing.T) { err := json.Unmarshal([]byte(msiJson), &msi) return msi, err }} - _, stream, err := Download(testctx, &downloader) - require.NoError(t, err, "File download failed") + _, stream, ewc := Download(testctx, &downloader) + require.NoError(t, ewc, "File download failed") defer stream.Close() bytes, err := ioutil.ReadAll(stream) diff --git a/pkg/download/downloader.go b/pkg/download/downloader.go index 622988e..1940375 100644 --- a/pkg/download/downloader.go +++ b/pkg/download/downloader.go @@ -7,6 +7,8 @@ import ( "net/http" "time" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/urlutil" "github.com/go-kit/kit/log" "github.com/pkg/errors" @@ -52,10 +54,10 @@ func HttpClientDo(request *http.Request) (*http.Response, error) { // 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. -func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, error) { +func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, *vmextension.ErrorWithClarification) { request, err := downloader.GetRequest() if err != nil { - return -1, nil, errors.Wrapf(err, "failed to create http request") + return -1, nil, vmextension.CreateWrappedErrorWithClarification(err, "failed to create http request") } requestID := request.Header.Get(xMsClientRequestIdHeaderName) if len(requestID) > 0 { @@ -65,7 +67,7 @@ func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, erro response, err := MakeHttpRequest(request) if err != nil { err = urlutil.RemoveUrlFromErr(err) - return -1, nil, errors.Wrapf(err, "http request failed") + return -1, nil, vmextension.CreateWrappedErrorWithClarification(err, "http request failed") } if response.StatusCode == http.StatusOK { @@ -73,6 +75,7 @@ func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, erro } errString := fmt.Sprintf("Status code %d while downloading blob '%s'. Use either a public script URI that points to .sh file, Azure storage blob SAS URI or storage blob accessible by a managed identity and retry. For more information, see https://aka.ms/RunCommandManagedLinux", response.StatusCode, request.URL.Opaque) + errCode := constants.FileDownload_FailedStatusCode requestId := response.Header.Get(xMsServiceRequestIdHeaderName) switch downloader.(type) { case *blobWithMsiToken: @@ -80,12 +83,14 @@ func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, erro case http.StatusNotFound: notFoundError := fmt.Sprintf("RunCommand failed to download the blob '%s' and received a response code '%s'. Make sure that the Azure blob and managed identity exist, and the identity has access to the storage blob's container with the 'Storage Blob Data Reader' role assignment. For a user-assigned identity, add it under the VM's identity. For more information, see https://aka.ms/RunCommandManagedLinux", request.URL.Opaque, response.Status) errString = fmt.Sprintf("%s: %s", MsiDownload404ErrorString, notFoundError) + errCode = constants.FileDownload_DoesNotExist case http.StatusForbidden, http.StatusUnauthorized, http.StatusBadRequest, http.StatusConflict: forbiddenError := fmt.Sprintf("RunCommand failed to download the blob '%s' and received a response code '%s'. Ensure that the managed identity has access to the storage blob's container with the 'Storage Blob Data Reader' role assignment. For a user-assigned identity, add it under the VM's identity. For more information, see https://aka.ms/RunCommandManagedLinux", request.URL.Opaque, response.Status) errString = fmt.Sprintf("%s: %s", MsiDownload403ErrorString, forbiddenError) + errCode = constants.FileDownload_AccessDenied } break default: @@ -95,30 +100,36 @@ func Download(ctx *log.Context, downloader Downloader) (int, io.ReadCloser, erro errString = fmt.Sprintf("RunCommand failed to download the file from %s because access was denied. Please fix the blob permissions and try again. The response code and message returned were: %q.", hostname, response.Status) + errCode = constants.FileDownload_AccessDenied break case http.StatusNotFound: errString = fmt.Sprintf("RunCommand failed to download the file from %s because it does not exist. Please create the blob and try again, the response code and message returned were: %q", hostname, response.Status) + errCode = constants.FileDownload_DoesNotExist case http.StatusBadRequest: errString = fmt.Sprintf("RunCommand failed to download the file from %s because parts of the request were incorrectly formatted, missing, and/or invalid. The response code and message returned were: %q", hostname, response.Status) + errCode = constants.FileDownload_BadRequest case http.StatusInternalServerError: errString = fmt.Sprintf("RunCommand failed to download the file from %s due to an issue with storage. The response code and message returned were: %q", hostname, response.Status) + errCode = constants.FileDownload_InternalServerError + default: errString = fmt.Sprintf("RunCommand failed to download the file from %s because the server returned a response code and message of %q Please verify the machine has network connectivity.", hostname, response.Status) + errCode = constants.FileDownload_NetworkingError } } if len(requestId) > 0 { errString += fmt.Sprintf(" (Service request ID: %s)", requestId) } - return response.StatusCode, nil, fmt.Errorf(errString) + return response.StatusCode, nil, vmextension.NewErrorWithClarificationPtr(errCode, errors.New(errString)) } diff --git a/pkg/download/downloader_test.go b/pkg/download/downloader_test.go index f910667..47390cd 100644 --- a/pkg/download/downloader_test.go +++ b/pkg/download/downloader_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/Azure/azure-extension-foundation/msi" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/download" "github.com/ahmetalpbalkan/go-httpbin" "github.com/go-kit/kit/log" @@ -57,14 +58,19 @@ func TestDownload_wrapsCommonErrorCodes(t *testing.T) { switch respCode { case http.StatusNotFound: require.Contains(t, err.Error(), "because it does not exist") + VerifyErrorClarification(t, constants.FileDownload_DoesNotExist, err) case http.StatusForbidden: require.Contains(t, err.Error(), "Please verify the machine has network connectivity") + VerifyErrorClarification(t, constants.FileDownload_NetworkingError, err) case http.StatusInternalServerError: require.Contains(t, err.Error(), "due to an issue with storage") + VerifyErrorClarification(t, constants.FileDownload_InternalServerError, err) case http.StatusBadRequest: require.Contains(t, err.Error(), "because parts of the request were incorrectly formatted, missing, and/or invalid") + VerifyErrorClarification(t, constants.FileDownload_BadRequest, err) case http.StatusUnauthorized: require.Contains(t, err.Error(), "because access was denied") + VerifyErrorClarification(t, constants.FileDownload_AccessDenied, err) } } } @@ -93,6 +99,7 @@ func TestDowload_msiDownloaderErrorMessage(t *testing.T) { require.Contains(t, err.Error(), "For more information, see https://aka.ms/RunCommandManagedLinux", "error string doesn't contain full message") require.Nil(t, body, "body is not nil for failed download") require.Equal(t, 404, returnCode, "return code was not 404") + VerifyErrorClarification(t, constants.FileDownload_DoesNotExist, err) msiDownloader403 := download.NewBlobWithMsiDownload(srv.URL+"/status/403", mockMsiProvider) returnCode, body, err = download.Download(testctx, msiDownloader403) @@ -100,6 +107,7 @@ func TestDowload_msiDownloaderErrorMessage(t *testing.T) { require.Contains(t, err.Error(), "For more information, see https://aka.ms/RunCommandManagedLinux", "error string doesn't contain full message") require.Nil(t, body, "body is not nil for failed download") require.Equal(t, 403, returnCode, "return code was not 403") + VerifyErrorClarification(t, constants.FileDownload_AccessDenied, err) // Should use default error message for any error code other than 400, 401, 403, 404, and 409 msiDownloader500 := download.NewBlobWithMsiDownload(srv.URL+"/status/500", mockMsiProvider) @@ -109,6 +117,7 @@ func TestDowload_msiDownloaderErrorMessage(t *testing.T) { require.Contains(t, err.Error(), "For more information, see https://aka.ms/RunCommandManagedLinux", "error string doesn't contain full message") require.Nil(t, body, "body is not nil for failed download") require.Equal(t, 500, returnCode, "return code was not 500") + VerifyErrorClarification(t, constants.FileDownload_FailedStatusCode, err) } @@ -116,8 +125,8 @@ func TestDownload_retrievesBody(t *testing.T) { srv := httptest.NewServer(httpbin.GetMux()) defer srv.Close() - _, body, err := download.Download(testctx, download.NewURLDownload(srv.URL+"/bytes/65536")) - require.Nil(t, err) + _, body, ewc := download.Download(testctx, download.NewURLDownload(srv.URL+"/bytes/65536")) + require.Nil(t, ewc) defer body.Close() b, err := ioutil.ReadAll(body) require.Nil(t, err) diff --git a/pkg/download/retry.go b/pkg/download/retry.go index 97fb314..3f61d31 100644 --- a/pkg/download/retry.go +++ b/pkg/download/retry.go @@ -7,8 +7,8 @@ import ( "net/http" "time" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/go-kit/kit/log" - "github.com/pkg/errors" ) // SleepFunc pauses the execution for at least duration d. @@ -32,8 +32,8 @@ const ( // closed on failures). If the retries do not succeed, the last error is returned. // // It sleeps in exponentially increasing durations between retries. -func WithRetries(ctx *log.Context, downloaders []Downloader, sf SleepFunc) (io.ReadCloser, error) { - var downloadErrors error +func WithRetries(ctx *log.Context, downloaders []Downloader, sf SleepFunc) (io.ReadCloser, *vmextension.ErrorWithClarification) { + var downloadError *vmextension.ErrorWithClarification for _, d := range downloaders { for n := 0; n < expRetryN; n++ { ctx := ctx.With("retry", n) @@ -42,13 +42,8 @@ func WithRetries(ctx *log.Context, downloaders []Downloader, sf SleepFunc) (io.R return out, nil } - if downloadErrors != nil { - downloadErrors = errors.Wrapf(downloadErrors, fmt.Sprintf("Attempt %d: %s ", n+1, err.Error())) - } else { - downloadErrors = err - } - - ctx.Log("error", err) + downloadError = err + ctx.Log(fmt.Sprintf("Attempt %d: %s ", n+1, downloadError.Error())) if out != nil { // we are not going to read this response body out.Close() @@ -74,7 +69,7 @@ func WithRetries(ctx *log.Context, downloaders []Downloader, sf SleepFunc) (io.R } } } - return nil, downloadErrors + return nil, downloadError } func isTransientHttpStatusCode(statusCode int) bool { diff --git a/pkg/download/save.go b/pkg/download/save.go index c04284d..f89972c 100644 --- a/pkg/download/save.go +++ b/pkg/download/save.go @@ -4,6 +4,8 @@ import ( "io" "os" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/go-kit/kit/log" "github.com/pkg/errors" ) @@ -16,19 +18,23 @@ const ( // given file. Directory of dst is not created by this function. If a file at // dst exists, it will be truncated. If a new file is created, mode is used to // set the permission bits. Written number of bytes are returned on success. -func SaveTo(ctx *log.Context, downloaders []Downloader, dst string, mode os.FileMode) (int64, error) { +func SaveTo(ctx *log.Context, downloaders []Downloader, dst string, mode os.FileMode) (int64, *vmextension.ErrorWithClarification) { f, err := os.OpenFile(dst, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode) if err != nil { - return 0, errors.Wrapf(err, "failed to open file for writing: %s", dst) + return 0, vmextension.NewErrorWithClarificationPtr(constants.FileDownload_OpenFileForWriteFailure, errors.Wrapf(err, "failed to open file for writing: %s", dst)) } defer f.Close() - body, err := WithRetries(ctx, downloaders, ActualSleep) - if err != nil { - return 0, errors.Wrapf(err, "failed to download file '%s'", dst) + body, ewc := WithRetries(ctx, downloaders, ActualSleep) + if ewc != nil { + return 0, ewc } defer body.Close() n, err := io.CopyBuffer(f, body, make([]byte, writeBufSize)) - return n, errors.Wrapf(err, "failed to write to file: %s", dst) + if err != nil { + return n, vmextension.NewErrorWithClarificationPtr(constants.FileDownload_WriteFileError, errors.Wrapf(err, "failed to write to file: %s", dst)) + } + + return n, nil } diff --git a/pkg/download/save_test.go b/pkg/download/save_test.go index d93d185..60ee726 100644 --- a/pkg/download/save_test.go +++ b/pkg/download/save_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "testing" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/download" "github.com/ahmetalpbalkan/go-httpbin" "github.com/stretchr/testify/require" @@ -21,6 +23,7 @@ func TestSaveTo_invalidDir(t *testing.T) { _, err := download.SaveTo(nopLog(), []download.Downloader{d}, "/nonexistent-dir/dst", 0600) require.Contains(t, err.Error(), "failed to open file for writing") + VerifyErrorClarification(t, constants.FileDownload_OpenFileForWriteFailure, err) } func TestSave(t *testing.T) { @@ -82,3 +85,8 @@ func TestSave_largeFile(t *testing.T) { require.Nil(t, err) require.EqualValues(t, size, fi.Size()) } + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +} diff --git a/pkg/preprocess/file.go b/pkg/preprocess/file.go index 7fb5e5f..037df33 100644 --- a/pkg/preprocess/file.go +++ b/pkg/preprocess/file.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" + "github.com/Azure/azure-extension-platform/vmextension" + "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/pkg/errors" ) @@ -24,19 +26,19 @@ var textExtensions = []string{ // IsTextFile is a best effort to determine if a file // is a script file (with a known file extension) or a // file that starts with a shebang (!#) -func IsTextFile(path string) (bool, error) { +func IsTextFile(path string) (bool, *vmextension.ErrorWithClarification) { if hasTextExtension(path) { return true, nil } f, err := os.Open(path) if err != nil { - return false, errors.Wrap(err, "failed to open file") + return false, vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToOpenFileForReading, errors.Wrap(err, "failed to open file")) } defer f.Close() b := make([]byte, peekLen) _, err = f.Read(b) if err != nil && err != io.EOF { - return false, errors.Wrap(err, "failed to read file") + return false, vmextension.NewErrorWithClarificationPtr(constants.Internal_FailedToReadFile, errors.Wrap(err, "failed to read file")) } return hasShebang(b), nil } diff --git a/pkg/preprocess/file_test.go b/pkg/preprocess/file_test.go index 3533de6..9107253 100644 --- a/pkg/preprocess/file_test.go +++ b/pkg/preprocess/file_test.go @@ -44,7 +44,7 @@ func TestIsTextFile(t *testing.T) { for f, exp := range files { out, err := IsTextFile(filepath.Join(testDataDir, f)) - require.NoError(t, err) + require.Nil(t, err) require.Equal(t, exp, out, "IsTextFile(%s)", f) } } diff --git a/pkg/servicehandler/servicehandler.go b/pkg/servicehandler/servicehandler.go index e3567fd..4670bfa 100644 --- a/pkg/servicehandler/servicehandler.go +++ b/pkg/servicehandler/servicehandler.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/go-kit/kit/log" "github.com/pkg/errors" @@ -82,25 +83,25 @@ func (handler *Handler) IsInstalled() (bool, error) { return handler.manager.IsUnitInstalled(handler.config.Name, handler.ctx) } -func (handler *Handler) Register(ctx *log.Context, unitConfigContent string) error { +func (handler *Handler) Register(ctx *log.Context, unitConfigContent string) *vmextension.ErrorWithClarification { err := handler.manager.RemoveUnitConfigurationFile(handler.config.Name, ctx) if err != nil && !os.IsNotExist(err) { - return fmt.Errorf("error while removing old unit configuration file: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_CouldNotRemoveOldUnitConfigFile, fmt.Errorf("error while removing old unit configuration file: %v", err)) } err = handler.manager.CreateUnitConfigurationFile(handler.config.Name, []byte(unitConfigContent), ctx) if err != nil { - return fmt.Errorf("error while creating unit configuration file: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_ErrorCreatingUnitConfig, fmt.Errorf("error while creating unit configuration file: %v", err)) } err = handler.DaemonReload() if err != nil { - return fmt.Errorf("error while reloading daemon worker: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_ErrorReloadingDaemonWorker, fmt.Errorf("error while reloading daemon worker: %v", err)) } err = handler.Enable() if err != nil { - return fmt.Errorf("error while enabling unit: %v", err) + return vmextension.NewErrorWithClarificationPtr(constants.Immediate_ErrorEnablingUnit, fmt.Errorf("error while enabling unit: %v", err)) } return nil diff --git a/pkg/servicehandler/servicehandler_test.go b/pkg/servicehandler/servicehandler_test.go index f5ed7e1..fd429d2 100644 --- a/pkg/servicehandler/servicehandler_test.go +++ b/pkg/servicehandler/servicehandler_test.go @@ -5,9 +5,11 @@ import ( "os" "testing" + "github.com/Azure/azure-extension-platform/vmextension" "github.com/Azure/run-command-handler-linux/internal/constants" "github.com/Azure/run-command-handler-linux/pkg/systemd" "github.com/go-kit/kit/log" + "github.com/stretchr/testify/require" ) const ( @@ -225,10 +227,7 @@ func TestHandlerRegisterFailsOnUnitConfigurationFileDeletion(t *testing.T) { // assert that the register call returns an error err := handler.Register(ctx, "") - - if err == nil { - t.Errorf("unexpected successful registration call") - } + VerifyErrorClarification(t, constants.Immediate_CouldNotRemoveOldUnitConfigFile, err) } func TestHandlerRegisterFailsOnUnitConfigurationFileCreation(t *testing.T) { @@ -246,10 +245,7 @@ func TestHandlerRegisterFailsOnUnitConfigurationFileCreation(t *testing.T) { // assert that the register call returns an error err := handler.Register(ctx, "") - - if err == nil { - t.Errorf("unexpected successful registration call") - } + VerifyErrorClarification(t, constants.Immediate_ErrorCreatingUnitConfig, err) } func TestHandlerRegisterFailsOnDaemonReload(t *testing.T) { @@ -268,9 +264,7 @@ func TestHandlerRegisterFailsOnDaemonReload(t *testing.T) { // assert that the register call returns an error err := handler.Register(ctx, "") - if err == nil { - t.Errorf("unexpected successful registration call") - } + VerifyErrorClarification(t, constants.Immediate_ErrorReloadingDaemonWorker, err) } func TestHandlerRegisterFailsOnEnable(t *testing.T) { @@ -289,9 +283,7 @@ func TestHandlerRegisterFailsOnEnable(t *testing.T) { // assert that the register call returns an error err := handler.Register(ctx, "") - if err == nil { - t.Errorf("unexpected successful registration call") - } + VerifyErrorClarification(t, constants.Immediate_ErrorEnablingUnit, err) } func TestHandlerSuccessfulDeRegister(t *testing.T) { @@ -771,3 +763,8 @@ func TestGetUnitConfigurationPathSystemD(t *testing.T) { t.Errorf("unexpected unit configuration path\nreturned path was %s", path) } } + +func VerifyErrorClarification(t *testing.T, expectedCode int, ewc *vmextension.ErrorWithClarification) { + require.NotNil(t, ewc, "No error returned when one was expected") + require.Equal(t, expectedCode, ewc.ErrorCode, "Expected error %d but received %d", expectedCode, ewc.ErrorCode) +}