Cover patch_linux.go by unit tests (#932)

* Cover patch_linux.go by unit tests

* Fix review comments

Author: iliatsuprik

agentendpoint/patch_linux.go

@@ -29,9 +29,15 @@ import (
 	"cloud.google.com/go/osconfig/agentendpoint/apiv1/agentendpointpb"
 )
 
+var (
+	runAptGetUpgrade = ospatch.RunAptGetUpgrade
+	runYumUpdate     = ospatch.RunYumUpdate
+	runZypperPatch   = ospatch.RunZypperPatch
+	retryPeriod      = 3 * time.Minute
+)
+
 func (r *patchTask) runUpdates(ctx context.Context) error {
 	var errs []string
-	const retryPeriod = 3 * time.Minute
 	// Check for both apt-get and dpkg-query to give us a clean signal.
 	if packages.AptExists && packages.DpkgQueryExists {
 		excludes, err := convertInputToExcludes(r.Task.GetPatchConfig().GetApt().GetExcludes())
@@ -48,7 +54,7 @@ func (r *patchTask) runUpdates(ctx context.Context) error {
 			opts = append(opts, ospatch.AptGetUpgradeType(packages.AptGetDistUpgrade))
 		}
 		clog.Debugf(ctx, "Installing APT package updates.")
-		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing APT package updates", func() error { return ospatch.RunAptGetUpgrade(ctx, opts...) }); err != nil {
+		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing APT package updates", func() error { return runAptGetUpgrade(ctx, opts...) }); err != nil {
 			errs = append(errs, err.Error())
 		}
 	}
@@ -65,7 +71,7 @@ func (r *patchTask) runUpdates(ctx context.Context) error {
 			ospatch.YumDryRun(r.Task.GetDryRun()),
 		}
 		clog.Debugf(ctx, "Installing YUM package updates.")
-		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing YUM package updates", func() error { return ospatch.RunYumUpdate(ctx, opts...) }); err != nil {
+		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing YUM package updates", func() error { return runYumUpdate(ctx, opts...) }); err != nil {
 			errs = append(errs, err.Error())
 		}
 	}
@@ -84,7 +90,7 @@ func (r *patchTask) runUpdates(ctx context.Context) error {
 			ospatch.ZypperUpdateDryrun(r.Task.GetDryRun()),
 		}
 		clog.Debugf(ctx, "Installing Zypper updates.")
-		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing Zypper updates", func() error { return ospatch.RunZypperPatch(ctx, opts...) }); err != nil {
+		if err := retryutil.RetryFunc(ctx, retryPeriod, "installing Zypper updates", func() error { return runZypperPatch(ctx, opts...) }); err != nil {
 			errs = append(errs, err.Error())
 		}
 	}

agentendpoint/patch_linux_test.go

@@ -15,44 +15,233 @@
 package agentendpoint
 
 import (
+	"context"
+	"errors"
 	"reflect"
 	"regexp"
 	"strings"
 	"testing"
+	"time"
 
+	"cloud.google.com/go/osconfig/agentendpoint/apiv1/agentendpointpb"
 	"github.com/GoogleCloudPlatform/osconfig/ospatch"
+	"github.com/GoogleCloudPlatform/osconfig/packages"
+	"github.com/GoogleCloudPlatform/osconfig/util/utiltest"
 )
 
 func TestExcludeConversion(t *testing.T) {
 	regex, _ := regexp.Compile("PackageName")
 	emptyRegex, _ := regexp.Compile("")
+	_, regexpErr := regexp.Compile("[a-z")
 
 	tests := []struct {
-		name  string
-		input []string
-		want  []*ospatch.Exclude
+		name    string
+		input   []string
+		want    []*ospatch.Exclude
+		wantErr error
 	}{
-		{name: "StrictStringConversion", input: []string{"PackageName"}, want: CreateStringExcludes("PackageName")},
-		{name: "MultipleStringConversion", input: []string{"PackageName1", "PackageName2"}, want: CreateStringExcludes("PackageName1", "PackageName2")},
-		{name: "RegexConversion", input: []string{"/PackageName/"}, want: []*ospatch.Exclude{ospatch.CreateRegexExclude(regex)}},
-		{name: "CornerCaseRegex", input: []string{"//"}, want: []*ospatch.Exclude{ospatch.CreateRegexExclude(emptyRegex)}},
-		{name: "CornerCaseStrictString", input: []string{"/"}, want: CreateStringExcludes("/")},
-		{name: "CornerCaseEmptyString", input: []string{""}, want: CreateStringExcludes("")},
+		{name: "Single package name, want one string exclude", input: []string{"PackageName"}, want: createStringExcludes("PackageName")},
+		{name: "Multiple package names, want multiple string excludes", input: []string{"PackageName1", "PackageName2"}, want: createStringExcludes("PackageName1", "PackageName2")},
+		{name: "Slash-wrapped value, want regex exclude", input: []string{"/PackageName/"}, want: []*ospatch.Exclude{ospatch.CreateRegexExclude(regex)}},
+		{name: "Empty regex //, want empty regex exclude", input: []string{"//"}, want: []*ospatch.Exclude{ospatch.CreateRegexExclude(emptyRegex)}},
+		{name: "Single slash, want string exclude", input: []string{"/"}, want: createStringExcludes("/")},
+		{name: "Empty string, want string exclude", input: []string{""}, want: createStringExcludes("")},
+		{name: "Invalid regex, want regex compile error", input: []string{"/[a-z/"}, wantErr: regexpErr},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			excludes, err := convertInputToExcludes(tt.input)
-			if err != nil {
-				t.Errorf("err = %v, want %v", err, nil)
-			}
+
 			if !reflect.DeepEqual(excludes, tt.want) {
 				t.Errorf("convertInputToExcludes() = %s, want = %s", toString(excludes), toString(tt.want))
 			}
+			utiltest.AssertErrorMatch(t, err, tt.wantErr)
+		})
+	}
+}
+
+func TestRunUpdates(t *testing.T) {
+	utiltest.OverrideVariable(t, &retryPeriod, 1*time.Millisecond)
+
+	mockAptSuccess := func(ctx context.Context, opts ...ospatch.AptGetUpgradeOption) error { return nil }
+	mockYumSuccess := func(ctx context.Context, opts ...ospatch.YumUpdateOption) error { return nil }
+	mockZypperSuccess := func(ctx context.Context, opts ...ospatch.ZypperPatchOption) error { return nil }
+	mockAptErr := func(ctx context.Context, opts ...ospatch.AptGetUpgradeOption) error { return errors.New("apt err") }
+	mockYumErr := func(ctx context.Context, opts ...ospatch.YumUpdateOption) error { return errors.New("yum err") }
+	mockZypperErr := func(ctx context.Context, opts ...ospatch.ZypperPatchOption) error { return errors.New("zypper err") }
+	_, regexpErr := regexp.Compile("[a-z")
+
+	tests := []struct {
+		name       string
+		setup      func(t *testing.T)
+		taskConfig *agentendpointpb.PatchConfig
+		wantErr    error
+	}{
+		{
+			name: "No package managers detected, want no patch operations attempted and no error",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{},
+			wantErr:    nil,
+		},
+		{
+			name: "Apt available, want apt-get dist-upgrade attempted and no error",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableApt(t, mockAptSuccess)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Apt: &agentendpointpb.AptSettings{Type: agentendpointpb.AptSettings_DIST},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "Apt available with invalid regex in excludes, want regex compile error and no upgrade attempted",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableApt(t, nil)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Apt: &agentendpointpb.AptSettings{Excludes: []string{"/[a-z/"}},
+			},
+			wantErr: regexpErr,
+		},
+		{
+			name: "Yum available, want yum update attempted and no error",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableYum(t, mockYumSuccess)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Yum: &agentendpointpb.YumSettings{
+					Security: true,
+					Minimal:  true,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "Zypper available, want zypper patch attempted and no error",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableZypper(t, mockZypperSuccess)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Zypper: &agentendpointpb.ZypperSettings{
+					WithUpdate:   true,
+					WithOptional: true,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "Yum available with invalid regex in excludes, want regex compile error and no update attempted",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableYum(t, nil)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Yum: &agentendpointpb.YumSettings{Excludes: []string{"/[a-z/"}},
+			},
+			wantErr: regexpErr,
+		},
+		{
+			name: "Zypper available with invalid regex in excludes, want regex compile error and no patch attempted",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableZypper(t, nil)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Zypper: &agentendpointpb.ZypperSettings{Excludes: []string{"/[a-z/"}},
+			},
+			wantErr: regexpErr,
+		},
+		{
+			name: "Yum available and yum update returns error, want that error returned",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableYum(t, mockYumErr)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Yum: &agentendpointpb.YumSettings{},
+			},
+			wantErr: errors.New("yum err"),
+		},
+		{
+			name: "All package managers available and each fails, want all three errors aggregated",
+			setup: func(t *testing.T) {
+				disableAllPackageManagers(t)
+				enableApt(t, mockAptErr)
+				enableYum(t, mockYumErr)
+				enableZypper(t, mockZypperErr)
+			},
+			taskConfig: &agentendpointpb.PatchConfig{
+				Apt:    &agentendpointpb.AptSettings{},
+				Yum:    &agentendpointpb.YumSettings{},
+				Zypper: &agentendpointpb.ZypperSettings{},
+			},
+			wantErr: errors.New("apt err,\nyum err,\nzypper err"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.setup(t)
+			task := initializePatchTask(tt.taskConfig)
+
+			err := task.runUpdates(context.Background())
+			utiltest.AssertErrorMatch(t, err, tt.wantErr)
 		})
 	}
 }
 
+func initializePatchTask(config *agentendpointpb.PatchConfig) *patchTask {
+	return &patchTask{
+		Task: &applyPatchesTask{
+			ApplyPatchesTask: &agentendpointpb.ApplyPatchesTask{
+				PatchConfig: config,
+			},
+		},
+	}
+}
+
+func disableAllPackageManagers(t *testing.T) {
+	t.Helper()
+	utiltest.OverrideVariable(t, &packages.DpkgQueryExists, false)
+	utiltest.OverrideVariable(t, &packages.YumExists, false)
+	utiltest.OverrideVariable(t, &packages.RPMQueryExists, false)
+	utiltest.OverrideVariable(t, &packages.ZypperExists, false)
+}
+
+func enableApt(t *testing.T, run func(ctx context.Context, opts ...ospatch.AptGetUpgradeOption) error) {
+	t.Helper()
+	utiltest.OverrideVariable(t, &packages.AptExists, true)
+	utiltest.OverrideVariable(t, &packages.DpkgQueryExists, true)
+	if run != nil {
+		utiltest.OverrideVariable(t, &runAptGetUpgrade, run)
+	}
+}
+
+func enableYum(t *testing.T, run func(ctx context.Context, opts ...ospatch.YumUpdateOption) error) {
+	t.Helper()
+	utiltest.OverrideVariable(t, &packages.YumExists, true)
+	utiltest.OverrideVariable(t, &packages.RPMQueryExists, true)
+	if run != nil {
+		utiltest.OverrideVariable(t, &runYumUpdate, run)
+	}
+}
+
+func enableZypper(t *testing.T, run func(ctx context.Context, opts ...ospatch.ZypperPatchOption) error) {
+	t.Helper()
+	utiltest.OverrideVariable(t, &packages.ZypperExists, true)
+	utiltest.OverrideVariable(t, &packages.RPMQueryExists, true)
+	if run != nil {
+		utiltest.OverrideVariable(t, &runZypperPatch, run)
+	}
+}
+
 func toString(excludes []*ospatch.Exclude) string {
 	results := make([]string, len(excludes))
 	for i, exc := range excludes {
@@ -62,7 +251,7 @@ func toString(excludes []*ospatch.Exclude) string {
 	return strings.Join(results, ",")
 }
 
-func CreateStringExcludes(pkgs ...string) []*ospatch.Exclude {
+func createStringExcludes(pkgs ...string) []*ospatch.Exclude {
 	excludes := make([]*ospatch.Exclude, len(pkgs))
 	for i := 0; i < len(pkgs); i++ {
 		pkg := pkgs[i]

policies/apt_test.go

@@ -301,7 +301,7 @@ func TestShouldUseSignedBy(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			defer utiltest.OverrideVariable(&osInfoProvider, tt.provider)()
+			utiltest.OverrideVariable(t, &osInfoProvider, tt.provider)
 			utiltest.AssertEquals(t, shouldUseSignedBy(ctx), tt.want)
 		})
 	}

util/utiltest/utiltest.go

@@ -137,10 +137,8 @@ func AssertFileContents(t *testing.T, filePath string, wantContents string) {
 }
 
 // OverrideVariable overrides the value of a variable and returns a function to restore it.
-func OverrideVariable[T any](ptr *T, val T) func() {
+func OverrideVariable[T any](t *testing.T, ptr *T, val T) {
 	original := *ptr
 	*ptr = val
-	return func() {
-		*ptr = original
-	}
+	t.Cleanup(func() { *ptr = original })
 }