diff --git a/cmd/agent/internal/daemon/agentupgrade.go b/cmd/agent/internal/daemon/agentupgrade.go index bc011d389..2fe9d7aae 100644 --- a/cmd/agent/internal/daemon/agentupgrade.go +++ b/cmd/agent/internal/daemon/agentupgrade.go @@ -18,9 +18,15 @@ import ( const ( agentUpgradeDownloadURLParameter = "downloadURL" + agentUpgradeSHA256Parameter = "sha256" agentUpgradeBinaryMode = 0o755 ) +type agentUpgradeRequest struct { + downloadURL string + sha256 string +} + // agentUpgradeSignal is the JSON payload for pending and failure signals. type agentUpgradeSignal struct { OperationName string `json:"operationName"` @@ -42,33 +48,40 @@ type fileAgentUpgradeSignalOperator struct { path string } -func agentUpgradeDownloadURL(parameters map[string]string) (string, error) { - downloadURL := strings.TrimSpace(parameters[agentUpgradeDownloadURLParameter]) - if downloadURL == "" { - return "", fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) +func parseAgentUpgradeRequest(parameters map[string]string) (agentUpgradeRequest, error) { + request := agentUpgradeRequest{ + downloadURL: strings.TrimSpace(parameters[agentUpgradeDownloadURLParameter]), + sha256: strings.TrimSpace(parameters[agentUpgradeSHA256Parameter]), + } + if request.downloadURL == "" { + return agentUpgradeRequest{}, fmt.Errorf("missing required parameter %q", agentUpgradeDownloadURLParameter) } - return downloadURL, nil + return request, nil } -func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, downloadURL string) error { +func upgradeDaemonBinary(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { paths, err := goalstates.ResolvedAgentUpgradePaths() if err != nil { return fmt.Errorf("resolve current daemon binary symlink: %w", err) } - targetPath := paths.NextTargetPath() - if err := agentbinary.InstallAndSwitchFromTarGz(ctx, downloadURL, paths, agentUpgradeBinaryMode); err != nil { - return err + layout := agentbinary.Layout{ + BinaryPath: paths.BinaryPath, + BluePath: paths.BluePath, + GreenPath: paths.GreenPath, + CurrentPath: paths.CurrentPath, + LastGoodPath: paths.LastGoodPath, } + _, err = agentbinary.InstallAndSwitchFromTarGz(ctx, log, layout, agentbinary.InstallOptions{ + DownloadURL: request.downloadURL, + ExpectedSHA256: request.sha256, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + Mode: agentUpgradeBinaryMode, + ExactMember: true, + }) - log.Info("staged upgraded daemon binary", - "url", downloadURL, - "previous", paths.CurrentTargetPath, - "current", targetPath, - ) - - return nil + return err } func newAgentUpgradeSignalOperator() (agentUpgradeSignalOperator, error) { diff --git a/cmd/agent/internal/daemon/agentupgrade_test.go b/cmd/agent/internal/daemon/agentupgrade_test.go index db2656877..bf52cebb9 100644 --- a/cmd/agent/internal/daemon/agentupgrade_test.go +++ b/cmd/agent/internal/daemon/agentupgrade_test.go @@ -4,46 +4,42 @@ package daemon import ( - "archive/tar" - "bytes" - "compress/gzip" - "context" - "fmt" - "io" - "log/slog" - "net/http" - "net/http/httptest" "os" "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/Azure/unbounded/pkg/agent/agentbinary" "github.com/Azure/unbounded/pkg/agent/goalstates" ) -func TestAgentUpgradeDownloadURL(t *testing.T) { +func TestParseAgentUpgradeRequest(t *testing.T) { t.Parallel() - downloadURL, err := agentUpgradeDownloadURL(map[string]string{ + request, err := parseAgentUpgradeRequest(map[string]string{ agentUpgradeDownloadURLParameter: " https://example.com/agent.tar.gz ", + agentUpgradeSHA256Parameter: " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ", }) require.NoError(t, err) - assert.Equal(t, "https://example.com/agent.tar.gz", downloadURL) + assert.Equal(t, "https://example.com/agent.tar.gz", request.downloadURL) + assert.Equal(t, testAgentUpgradeSHA256, request.sha256) - _, err = agentUpgradeDownloadURL(nil) + _, err = parseAgentUpgradeRequest(nil) require.Error(t, err) assert.Contains(t, err.Error(), agentUpgradeDownloadURLParameter) + + request, err = parseAgentUpgradeRequest(map[string]string{ + agentUpgradeDownloadURLParameter: "http://example.com/agent.tar.gz", + }) + require.NoError(t, err) + assert.Empty(t, request.sha256) } -func TestAgentUpgradeSignalOperator_RecordFailure(t *testing.T) { +func TestAgentUpgradeSignalOperatorRecordFailure(t *testing.T) { t.Parallel() - dir := t.TempDir() - signalPath := filepath.Join(dir, "agent-upgrade-signal") + signalPath := filepath.Join(t.TempDir(), "agent-upgrade-signal") signals := newAgentUpgradeSignalOperatorForPath(signalPath) require.NoError(t, signals.RecordPending("op-1", 7)) @@ -54,11 +50,10 @@ func TestAgentUpgradeSignalOperator_RecordFailure(t *testing.T) { assert.JSONEq(t, `{"operationName":"op-1","observedMachineGeneration":7,"failureMessage":"rolled back"}`, string(data)) } -func TestAgentUpgradeSignalOperator_ReadRejectsNonJSON(t *testing.T) { +func TestAgentUpgradeSignalOperatorReadRejectsNonJSON(t *testing.T) { t.Parallel() - dir := t.TempDir() - signalPath := filepath.Join(dir, "agent-upgrade-signal") + signalPath := filepath.Join(t.TempDir(), "agent-upgrade-signal") signals := newAgentUpgradeSignalOperatorForPath(signalPath) require.NoError(t, os.WriteFile(signalPath, []byte("op-1\n"), 0o600)) @@ -67,169 +62,6 @@ func TestAgentUpgradeSignalOperator_ReadRejectsNonJSON(t *testing.T) { assert.Contains(t, err.Error(), "decode AgentUpgrade signal") } -func TestUpgradeDaemonBinary(t *testing.T) { - dir := t.TempDir() - legacyPath := filepath.Join(dir, "unbounded-agent") - currentPath := filepath.Join(dir, "unbounded-agent-current") - lastGoodPath := filepath.Join(dir, "unbounded-agent-last-good") - bluePath := filepath.Join(dir, "unbounded-agent-blue") - greenPath := filepath.Join(dir, "unbounded-agent-green") - - require.NoError(t, os.WriteFile(legacyPath, []byte("legacy"), 0o755)) - require.NoError(t, os.Symlink(legacyPath, currentPath)) - - t.Setenv(goalstates.EnvDaemonBinary, legacyPath) - t.Setenv(goalstates.EnvDaemonBinaryCurrent, currentPath) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, lastGoodPath) - t.Setenv(goalstates.EnvDaemonBinaryBlue, bluePath) - t.Setenv(goalstates.EnvDaemonBinaryGreen, greenPath) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "application/gzip") - w.WriteHeader(http.StatusOK) - require.NoError(t, writeAgentArchive(w, agentArchiveScript("new-agent-binary", 0))) - })) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - target, err := filepath.EvalSymlinks(currentPath) - require.NoError(t, err) - assert.Equal(t, bluePath, target) - - lastGoodTarget, err := filepath.EvalSymlinks(lastGoodPath) - require.NoError(t, err) - assert.Equal(t, legacyPath, lastGoodTarget) - - newData, err := os.ReadFile(bluePath) - require.NoError(t, err) - assert.Equal(t, agentArchiveScript("new-agent-binary", 0), newData) -} - -func TestUpgradeDaemonBinary_AlternatesFromBlueToGreen(t *testing.T) { - dir := t.TempDir() - currentPath := filepath.Join(dir, "unbounded-agent-current") - lastGoodPath := filepath.Join(dir, "unbounded-agent-last-good") - bluePath := filepath.Join(dir, "unbounded-agent-blue") - greenPath := filepath.Join(dir, "unbounded-agent-green") - - require.NoError(t, os.WriteFile(bluePath, []byte("blue"), 0o755)) - require.NoError(t, os.Symlink(bluePath, currentPath)) - - t.Setenv(goalstates.EnvDaemonBinaryCurrent, currentPath) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, lastGoodPath) - t.Setenv(goalstates.EnvDaemonBinaryBlue, bluePath) - t.Setenv(goalstates.EnvDaemonBinaryGreen, greenPath) - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - require.NoError(t, writeAgentArchive(w, agentArchiveScript("green", 0))) - })) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - target, err := filepath.EvalSymlinks(currentPath) - require.NoError(t, err) - assert.Equal(t, greenPath, target) - - lastGoodTarget, err := filepath.EvalSymlinks(lastGoodPath) - require.NoError(t, err) - assert.Equal(t, bluePath, lastGoodTarget) -} - -func TestUpgradeDaemonBinary_SequentialSuccesses(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 0)}, - {binary: agentArchiveScript("agent-b", 0)}, - }) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - assertSymlinkTarget(t, paths.current, paths.green) - assertSymlinkTarget(t, paths.lastGood, paths.blue) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 0))) - assertFileContent(t, paths.green, string(agentArchiveScript("agent-b", 0))) -} - -func TestUpgradeDaemonBinary_SequentialSuccessThenFailure(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 0)}, - {status: http.StatusInternalServerError}, - }) - t.Cleanup(server.Close) - - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 0))) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_SequentialFailureThenFailure(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {status: http.StatusInternalServerError}, - {status: http.StatusInternalServerError}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.legacy) - assert.NoFileExists(t, paths.lastGood) - assert.NoFileExists(t, paths.blue) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_SequentialFailureThenSuccess(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {status: http.StatusInternalServerError}, - {binary: agentArchiveScript("agent-b", 0)}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - require.NoError(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.blue) - assertSymlinkTarget(t, paths.lastGood, paths.legacy) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-b", 0))) - assert.NoFileExists(t, paths.green) -} - -func TestUpgradeDaemonBinary_RejectsBrokenBinary(t *testing.T) { - paths := setupDaemonBinaryTest(t) - server := newAgentArchiveSequenceServer(t, []archiveResponse{ - {binary: agentArchiveScript("agent-a", 42)}, - }) - t.Cleanup(server.Close) - - require.Error(t, upgradeDaemonBinary(context.Background(), slog.Default(), server.URL)) - - assertSymlinkTarget(t, paths.current, paths.legacy) - assert.NoFileExists(t, paths.lastGood) - assertFileContent(t, paths.blue, string(agentArchiveScript("agent-a", 42))) - assert.NoFileExists(t, paths.green) -} - -func TestDownloadAgentBinaryFromTarGz_RejectsUnsupportedScheme(t *testing.T) { - t.Parallel() - - err := agentbinary.InstallFromTarGz(context.Background(), "file:///tmp/unbounded-agent.tar.gz", filepath.Join(t.TempDir(), "agent"), goalstates.AgentUpgradeBinaryName, 0o755) - require.Error(t, err) - assert.Contains(t, err.Error(), "unsupported agent download URL scheme") -} - func TestAgentUpgradePathsInitialDaemonBinaryTarget(t *testing.T) { t.Parallel() @@ -261,112 +93,3 @@ func TestAgentUpgradePathsInitialDaemonBinaryTarget(t *testing.T) { _, err = paths.InitialDaemonBinaryTarget() require.Error(t, err) } - -type daemonBinaryTestPaths struct { - legacy string - current string - lastGood string - blue string - green string -} - -func setupDaemonBinaryTest(t *testing.T) daemonBinaryTestPaths { - t.Helper() - - paths := setupDaemonBinaryTestWithoutLinks(t) - require.NoError(t, os.WriteFile(paths.legacy, []byte("legacy"), 0o755)) - require.NoError(t, os.Symlink(paths.legacy, paths.current)) - - return paths -} - -func setupDaemonBinaryTestWithoutLinks(t *testing.T) daemonBinaryTestPaths { - t.Helper() - - dir := t.TempDir() - paths := daemonBinaryTestPaths{ - legacy: filepath.Join(dir, "unbounded-agent"), - current: filepath.Join(dir, "unbounded-agent-current"), - lastGood: filepath.Join(dir, "unbounded-agent-last-good"), - blue: filepath.Join(dir, "unbounded-agent-blue"), - green: filepath.Join(dir, "unbounded-agent-green"), - } - - t.Setenv(goalstates.EnvDaemonBinary, paths.legacy) - t.Setenv(goalstates.EnvDaemonBinaryCurrent, paths.current) - t.Setenv(goalstates.EnvDaemonBinaryLastGood, paths.lastGood) - t.Setenv(goalstates.EnvDaemonBinaryBlue, paths.blue) - t.Setenv(goalstates.EnvDaemonBinaryGreen, paths.green) - - return paths -} - -type archiveResponse struct { - binary []byte - status int -} - -func newAgentArchiveSequenceServer(t *testing.T, responses []archiveResponse) *httptest.Server { - t.Helper() - - next := 0 - - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - require.Less(t, next, len(responses)) - response := responses[next] - next++ - - if response.status != 0 { - http.Error(w, "failed", response.status) - return - } - - w.Header().Set("Content-Type", "application/gzip") - require.NoError(t, writeAgentArchive(w, response.binary)) - })) -} - -func assertSymlinkTarget(t *testing.T, linkPath, expectedTarget string) { - t.Helper() - - target, err := filepath.EvalSymlinks(linkPath) - require.NoError(t, err) - assert.Equal(t, expectedTarget, target) -} - -func assertFileContent(t *testing.T, path, expected string) { - t.Helper() - - data, err := os.ReadFile(path) - require.NoError(t, err) - assert.Equal(t, expected, string(data)) -} - -func writeAgentArchive(w io.Writer, binary []byte) error { - gz := gzip.NewWriter(w) - defer gz.Close() - - tw := tar.NewWriter(gz) - defer tw.Close() - - header := &tar.Header{ - Name: "unbounded-agent", - Mode: 0o755, - Size: int64(len(binary)), - } - if err := tw.WriteHeader(header); err != nil { - return err - } - - _, err := io.Copy(tw, bytes.NewReader(binary)) - - return err -} - -func agentArchiveScript(version string, exitCode int) []byte { - return []byte(fmt.Sprintf("#!/bin/sh\nprintf '%%s\\n' %s\nexit %d\n", shellQuote(version), exitCode)) -} - -func shellQuote(value string) string { - return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" -} diff --git a/cmd/agent/internal/daemon/controller_machineoperation.go b/cmd/agent/internal/daemon/controller_machineoperation.go index ed2f631da..1c826dd47 100644 --- a/cmd/agent/internal/daemon/controller_machineoperation.go +++ b/cmd/agent/internal/daemon/controller_machineoperation.go @@ -59,14 +59,14 @@ func (t *machineOperationTarget) reconcileAgentUpgrade(ctx context.Context, stor return ctrl.Result{}, err } - downloadURL, err := agentUpgradeDownloadURL(op.Parameters) + request, err := parseAgentUpgradeRequest(op.Parameters) if err != nil { return ctrl.Result{}, store.Finish(ctx, op, daemon.MachineOperationResult[int64]{Phase: v1alpha3.OperationPhaseFailed, Reason: "InvalidParameters", Message: err.Error()}) } - t.log.Info("staging AgentUpgrade binary", "operation", op.Name, "url", downloadURL) + t.log.Info("staging AgentUpgrade binary", "operation", op.Name) - if err := t.nodeOperator.StageAgentUpgrade(ctx, t.log, downloadURL); err != nil { + if err := t.nodeOperator.StageAgentUpgrade(ctx, t.log, request); err != nil { return finishFailedMachineOperation(ctx, store, op, err) } diff --git a/cmd/agent/internal/daemon/controller_test.go b/cmd/agent/internal/daemon/controller_test.go index 0a4c60cab..694264817 100644 --- a/cmd/agent/internal/daemon/controller_test.go +++ b/cmd/agent/internal/daemon/controller_test.go @@ -26,6 +26,8 @@ import ( "github.com/Azure/unbounded/pkg/agent/goalstates" ) +const testAgentUpgradeSHA256 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + type fakeNodeOperator struct { active *ActiveMachine findErr error @@ -46,6 +48,7 @@ type fakeNodeOperator struct { stageUpgradeCalled bool stageUpgradeURL string + stageUpgradeSHA256 string stageUpgradeErr error restartAgentCalled bool @@ -91,9 +94,10 @@ func (op *fakeNodeOperator) RepaveNode( return op.repaveErr } -func (op *fakeNodeOperator) StageAgentUpgrade(_ context.Context, _ *slog.Logger, downloadURL string) error { +func (op *fakeNodeOperator) StageAgentUpgrade(_ context.Context, _ *slog.Logger, request agentUpgradeRequest) error { op.stageUpgradeCalled = true - op.stageUpgradeURL = downloadURL + op.stageUpgradeURL = request.downloadURL + op.stageUpgradeSHA256 = request.sha256 return op.stageUpgradeErr } @@ -238,6 +242,7 @@ func TestReconcileAgentUpgrade_Complete(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } @@ -250,6 +255,7 @@ func TestReconcileAgentUpgrade_Complete(t *testing.T) { require.NoError(t, err) assert.True(t, op.stageUpgradeCalled) assert.Equal(t, "https://example.com/unbounded-agent.tar.gz", op.stageUpgradeURL) + assert.Equal(t, testAgentUpgradeSHA256, op.stageUpgradeSHA256) assert.True(t, op.restartAgentCalled) var updated v1alpha3.MachineOperation @@ -307,6 +313,7 @@ func TestReconcileAgentUpgrade_Failed(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } @@ -336,6 +343,7 @@ func TestReconcileAgentUpgrade_RestartFailureFailsOperation(t *testing.T) { OperationKind: v1alpha3.OperationAgentUpgrade, Parameters: map[string]string{ agentUpgradeDownloadURLParameter: "https://example.com/unbounded-agent.tar.gz", + agentUpgradeSHA256Parameter: testAgentUpgradeSHA256, }, }, } diff --git a/cmd/agent/internal/daemon/nodeoperator.go b/cmd/agent/internal/daemon/nodeoperator.go index 5ed45a161..ce3563960 100644 --- a/cmd/agent/internal/daemon/nodeoperator.go +++ b/cmd/agent/internal/daemon/nodeoperator.go @@ -56,7 +56,7 @@ type nodeOperator interface { // not perform Kubernetes eviction or CNI-specific dataplane cleanup itself. RepaveNode(context.Context, *slog.Logger, *ActiveMachine, *provision.UnboundedAgentConfig) error // StageAgentUpgrade stages a new host-side agent binary. - StageAgentUpgrade(context.Context, *slog.Logger, string) error + StageAgentUpgrade(context.Context, *slog.Logger, agentUpgradeRequest) error // RestartAgentDaemon restarts the host-side agent daemon after an upgrade // operation has been recorded as complete. RestartAgentDaemon(context.Context, *slog.Logger) error @@ -264,8 +264,8 @@ func (nspawnNodeOperator) RepaveNode( return nil } -func (nspawnNodeOperator) StageAgentUpgrade(ctx context.Context, log *slog.Logger, downloadURL string) error { - return upgradeDaemonBinary(ctx, log, downloadURL) +func (nspawnNodeOperator) StageAgentUpgrade(ctx context.Context, log *slog.Logger, request agentUpgradeRequest) error { + return upgradeDaemonBinary(ctx, log, request) } func (nspawnNodeOperator) RestartAgentDaemon(ctx context.Context, log *slog.Logger) error { diff --git a/cmd/kubectl-unbounded/app/machine_operation_aliases.go b/cmd/kubectl-unbounded/app/machine_operation_aliases.go index 9fed2402b..e68636f3b 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_aliases.go +++ b/cmd/kubectl-unbounded/app/machine_operation_aliases.go @@ -71,7 +71,10 @@ func newMachinePowerOnCommand(rt *machineCommandRuntime) *cobra.Command { } func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { - var downloadURL string + var ( + downloadURL string + sha256Digest string + ) cmd := newMachineOperationAliasCommand( rt, @@ -81,7 +84,8 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { "agent-upgrade", ) - cmd.Flags().StringVar(&downloadURL, "download-url", "", "URL of the unbounded-agent release tarball") + cmd.Flags().StringVar(&downloadURL, "download-url", "", "HTTP or HTTPS URL of the unbounded-agent release tarball") + cmd.Flags().StringVar(&sha256Digest, "sha256", "", "Optional SHA-256 digest of the release tarball") oldRunE := cmd.RunE cmd.RunE = func(cmd *cobra.Command, args []string) error { @@ -89,9 +93,12 @@ func newMachineAgentUpgradeCommand(rt *machineCommandRuntime) *cobra.Command { return fmt.Errorf("--download-url is required") } - cmd.SetContext(context.WithValue(cmd.Context(), machineOperationParametersKey{}, map[string]string{ - "downloadURL": downloadURL, - })) + parameters := map[string]string{"downloadURL": downloadURL} + if sha256Digest != "" { + parameters["sha256"] = sha256Digest + } + + cmd.SetContext(context.WithValue(cmd.Context(), machineOperationParametersKey{}, parameters)) return oldRunE(cmd, args) } diff --git a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go index db8039c88..ea0cf76e3 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_e2e_test.go @@ -48,6 +48,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { "--kind", string(v1alpha3.OperationAgentUpgrade), "--machine", "worker-01", "--param", "downloadURL=https://example.com/agent.tar.gz", + "--param", "sha256="+testAgentUpgradeSHA256, "--ttl", "900", ) require.NoError(t, err) @@ -57,6 +58,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { require.Nil(t, op.Spec.MachineSelector) require.Equal(t, v1alpha3.OperationAgentUpgrade, op.Spec.OperationKind) require.Equal(t, "https://example.com/agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) require.NotNil(t, op.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(900), *op.Spec.TTLSecondsAfterFinished) }) @@ -102,7 +104,7 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { ctx, rt, "machine", "agent-upgrade", "worker-01", "--operation-name", "worker-01-agent-upgrade", - "--download-url", "https://example.com/new-agent.tar.gz", + "--download-url", "http://example.com/new-agent.tar.gz", "--wait=false", ) require.NoError(t, err) @@ -110,7 +112,8 @@ func TestMachineOperationCommandsEndToEnd(t *testing.T) { assertMachineOperation(t, ctx, c, "worker-01-agent-upgrade", func(op v1alpha3.MachineOperation) { require.Equal(t, "worker-01", op.Spec.MachineRef) require.Equal(t, v1alpha3.OperationAgentUpgrade, op.Spec.OperationKind) - require.Equal(t, "https://example.com/new-agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, "http://example.com/new-agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.NotContains(t, op.Spec.Parameters, "sha256") require.NotNil(t, op.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(defaultTTLSeconds), *op.Spec.TTLSecondsAfterFinished) require.Len(t, op.OwnerReferences, 1) diff --git a/cmd/kubectl-unbounded/app/machine_operation_test.go b/cmd/kubectl-unbounded/app/machine_operation_test.go index 87325d2c4..4b2220d43 100644 --- a/cmd/kubectl-unbounded/app/machine_operation_test.go +++ b/cmd/kubectl-unbounded/app/machine_operation_test.go @@ -23,6 +23,8 @@ import ( v1alpha3 "github.com/Azure/unbounded/api/machina/v1alpha3" ) +const testAgentUpgradeSHA256 = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + func TestBuildMachineOperationWithMachineRef(t *testing.T) { t.Parallel() @@ -76,7 +78,7 @@ func TestBuildMachineOperationParameters(t *testing.T) { name: "upgrade-worker-01", kind: v1alpha3.OperationAgentUpgrade, machine: "worker-01", - parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz", "sha256=" + testAgentUpgradeSHA256}, output: operationOutputName, dryRun: dryRunNone, } @@ -86,6 +88,7 @@ func TestBuildMachineOperationParameters(t *testing.T) { op, err := opts.build() require.NoError(t, err) require.Equal(t, "https://example.com/agent.tar.gz", op.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, op.Spec.Parameters["sha256"]) } func TestValidateMachineOperationRequiresTarget(t *testing.T) { @@ -119,6 +122,22 @@ func TestValidateAgentUpgradeRequiresDownloadURL(t *testing.T) { require.Contains(t, err.Error(), "downloadURL") } +func TestValidateAgentUpgradeRequiresSHA256(t *testing.T) { + t.Parallel() + + opts := &machineOperationCreateOptions{ + name: "upgrade-worker-01", + kind: v1alpha3.OperationAgentUpgrade, + machine: "worker-01", + parameterArgs: []string{"downloadURL=http://example.com/agent.tar.gz"}, + output: operationOutputName, + dryRun: dryRunNone, + } + + // SHA-256 is optional when the operation source is trusted. + require.NoError(t, opts.validate()) +} + func TestValidateWaitRejectsStructuredOutput(t *testing.T) { t.Parallel() @@ -173,6 +192,7 @@ func TestMachineOperationCreateCommandDryRunYAML(t *testing.T) { "--kind", string(v1alpha3.OperationAgentUpgrade), "--machine", "worker-01", "--param", "downloadURL=https://example.com/agent.tar.gz", + "--param", "sha256="+testAgentUpgradeSHA256, "--ttl", "900", "--dry-run=client", "-o", "yaml", @@ -183,6 +203,7 @@ func TestMachineOperationCreateCommandDryRunYAML(t *testing.T) { require.Contains(t, out, "operationKind: AgentUpgrade") require.Contains(t, out, "machineRef: worker-01") require.Contains(t, out, "downloadURL: https://example.com/agent.tar.gz") + require.Contains(t, out, "sha256: "+testAgentUpgradeSHA256) require.Contains(t, out, "ttlSecondsAfterFinished: 900") } @@ -214,7 +235,7 @@ func TestMachineOperationCreateSmokeCreatesMachineRefOperation(t *testing.T) { name: "upgrade-worker-01", kind: v1alpha3.OperationAgentUpgrade, machine: "worker-01", - parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz"}, + parameterArgs: []string{"downloadURL=https://example.com/agent.tar.gz", "sha256=" + testAgentUpgradeSHA256}, ttlSeconds: 900, output: operationOutputName, dryRun: dryRunNone, @@ -233,6 +254,7 @@ func TestMachineOperationCreateSmokeCreatesMachineRefOperation(t *testing.T) { require.Nil(t, got.Spec.MachineSelector) require.Equal(t, v1alpha3.OperationAgentUpgrade, got.Spec.OperationKind) require.Equal(t, "https://example.com/agent.tar.gz", got.Spec.Parameters["downloadURL"]) + require.Equal(t, testAgentUpgradeSHA256, got.Spec.Parameters["sha256"]) require.NotNil(t, got.Spec.TTLSecondsAfterFinished) require.Equal(t, int32(900), *got.Spec.TTLSecondsAfterFinished) } diff --git a/designs/agent-upgrade.md b/designs/agent-upgrade.md index 3dd2181d3..c69a23dcf 100644 --- a/designs/agent-upgrade.md +++ b/designs/agent-upgrade.md @@ -62,7 +62,7 @@ Pending MachineOperation v Validate parameters | - +-- missing downloadURL -------------------------> Failed + +-- missing/invalid HTTP(S) URL -----------------> Failed | v Mark InProgress @@ -96,18 +96,20 @@ Old process exits, new daemon starts ## Staging and switching -The daemon reads `spec.parameters["downloadURL"]` from the -`MachineOperation`. It logs the URL, resolves the current binary target, and -calls `agentbinary.InstallAndSwitchFromTarGz`. +The daemon reads `spec.parameters["downloadURL"]` and the optional +`spec.parameters["sha256"]` from the `MachineOperation`, resolves the current +binary target, and calls `agentbinary.InstallAndSwitchFromTarGz`. +Logs and errors omit URL query and fragment data. `InstallAndSwitchFromTarGz` performs the upgrade as one logical operation: -1. Download the tarball. -2. Extract the `unbounded-agent` entry into `NextTargetPath()`. -3. Reject an empty agent entry. -4. Run `unbounded-agent version` against the staged binary. -5. Update `LastGoodPath` to the previous `CurrentTargetPath`. -6. Update `CurrentPath` to the staged binary. +1. Require an HTTP or HTTPS URL and verify the compressed-archive SHA-256 when provided. +2. Download the tarball within the configured size bound. +3. Require the archive to contain only the exact `unbounded-agent` entry. +4. Bound decompression and atomically install the inactive slot. +5. Run `unbounded-agent version` against the staged binary without exposing output. +6. If the inactive slot is last-good, protect the running binary through `LastGoodPath` before replacing it; otherwise defer the last-good update until candidate verification succeeds. +7. Atomically update `CurrentPath` to the staged binary. Symlink replacement uses `renameio.Symlink` through `utilio`, so each link is replaced atomically after parent directory creation. @@ -167,9 +169,10 @@ startup signal path. | Failure | Operation status | Binary state | |---------|------------------|--------------| | Missing `downloadURL` | `Failed`, `InvalidParameters` | No link changes. | -| Download or extraction failure | `Failed`, `ExecutionFailed` | No link changes after failure. | -| Empty archive entry | `Failed`, `ExecutionFailed` | No link changes after failure. | -| Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current and last-good remain unchanged. | +| Unsupported URL or digest mismatch | `Failed`, `InvalidParameters` or `ExecutionFailed` | No current link change. | +| Download or extraction failure | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | +| Empty archive entry | `Failed`, `ExecutionFailed` | Current remains unchanged. Last-good changes to current only when needed to protect an inactive slot that it referenced. | +| Staged binary fails `version` | `Failed`, `ExecutionFailed` | Current remains unchanged. A distinct last-good target remains unchanged. | | Restart command fails | `Failed` | Signal is cleared. Links may already point to the staged binary. | | Upgraded daemon fails under systemd | `Failed`, `DaemonFailed` | Recovery restores current to last-good. | diff --git a/docs/content/guides/operations/agent-operations.md b/docs/content/guides/operations/agent-operations.md index 393ff6828..0e708c6f5 100644 --- a/docs/content/guides/operations/agent-operations.md +++ b/docs/content/guides/operations/agent-operations.md @@ -10,8 +10,8 @@ handled by the agent itself. ## AgentUpgrade Replaces the host agent binary using blue-green staging with automatic rollback. -The operation requires a `downloadURL` parameter pointing to an agent release -tarball. +The operation requires a `downloadURL` parameter pointing to an HTTP or HTTPS +agent release tarball. An optional `sha256` parameter verifies the compressed archive digest. ```yaml apiVersion: unbounded-cloud.io/v1alpha3 @@ -23,6 +23,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` ```bash @@ -44,8 +45,8 @@ active at a time. **Staging:** -1. Downloads the release tarball from `downloadURL`. -2. Extracts the agent binary into the inactive slot. +1. Downloads the release tarball from `downloadURL` and verifies `sha256` when provided. +2. Requires the bounded archive to contain only the exact `unbounded-agent` member. 3. Runs `unbounded-agent version` against the staged binary as a binary validation check. If this fails, the operation is marked `Failed` and the current binary is unchanged. diff --git a/docs/content/guides/operations/automation.md b/docs/content/guides/operations/automation.md index ae2efac7e..da7d1beff 100644 --- a/docs/content/guides/operations/automation.md +++ b/docs/content/guides/operations/automation.md @@ -139,6 +139,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-v1.2.0-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ttlSecondsAfterFinished: 7200 EOF @@ -170,6 +171,7 @@ spec: operationKind: AgentUpgrade parameters: downloadURL: https://example.com/releases/unbounded-agent-v1.2.0-linux-amd64.tar.gz + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` ### Node Upgrade via Recreation diff --git a/docs/content/reference/cli.md b/docs/content/reference/cli.md index ff06f9758..7d75d8779 100644 --- a/docs/content/reference/cli.md +++ b/docs/content/reference/cli.md @@ -319,7 +319,7 @@ Create a `MachineOperation`. | `--kind` | string | Operation kind: `NodeReboot`, `AgentUpgrade`, `AgentReset`, `HostReboot`, `HostPowerOff`, `HostPowerOn`, or `HostReplace` | | `--machine` or `--selector` | string | Target one Machine by name or select Machines by label selector | -`AgentUpgrade` also requires `--param downloadURL=`. +`AgentUpgrade` requires `--param downloadURL=` and optionally accepts `--param sha256=`. #### Optional Flags @@ -372,7 +372,8 @@ Upgrade the agent: kubectl unbounded machine operation create upgrade-worker-01 \ --kind AgentUpgrade \ --machine worker-01 \ - --param downloadURL=https://example.com/unbounded-agent-linux-amd64.tar.gz + --param downloadURL=https://example.com/unbounded-agent-linux-amd64.tar.gz \ + --param sha256=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef ``` Selector support is implemented at the CRD level. Agent operations support @@ -411,7 +412,7 @@ name, default `--ttl 300`, and `--wait=true`. | `kubectl unbounded machine host-reboot NAME` | `HostReboot` | Reboots or power-cycles the host through the owning backend. | | `kubectl unbounded machine power-off NAME` | `HostPowerOff` | Powers off the host. | | `kubectl unbounded machine power-on NAME` | `HostPowerOn` | Powers on the host. | -| `kubectl unbounded machine agent-upgrade NAME --download-url URL` | `AgentUpgrade` | Upgrades the host-side agent binary. | +| `kubectl unbounded machine agent-upgrade NAME --download-url URL [--sha256 DIGEST]` | `AgentUpgrade` | Upgrades the host-side agent binary. | | `kubectl unbounded machine agent-reset NAME --force` | `AgentReset` | Removes the agent and managed resources from the host. Requires confirmation unless `--force` is set. | | `kubectl unbounded machine replace NAME --force` | `HostReplace` | Destructively replaces the host. Requires confirmation unless `--force` is set. | diff --git a/docs/content/reference/machina-crd.md b/docs/content/reference/machina-crd.md index 9f3f90527..1ff031a8b 100644 --- a/docs/content/reference/machina-crd.md +++ b/docs/content/reference/machina-crd.md @@ -196,7 +196,7 @@ spec: | `status.targets` | []TargetStatus | No | Per-Machine target status snapshot used by host operation controllers. | | `status.conditions` | []Condition | No | Operation conditions. `Completed` tracks terminal state. `BootLoaderDownloaded=True` is latched by metalman when a target first downloads the initial PXE boot loader, usually over TFTP. `BootImageWritten` starts as `Unknown` for metalman `HostReplace`, transitions to `False` when the PXE installer requests `disk.img.gz`, and transitions to `True` when the existing `/pxe/disable` completion signal is received. `CloudInitDone` starts as `Unknown`, transitions to `False` when first-boot cloud-init starts, and transitions to `True` on final cloud-init success or `False` with reason `Failed` and a summarized error when cloud-init reports a failure. | -`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL`. The URL must point to an `unbounded-agent` release tarball; the agent stages it as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. +`AgentUpgrade` is handled by the in-host agent and requires `spec.parameters.downloadURL`. The URL may use HTTP or HTTPS and must point to an `unbounded-agent` release tarball. The optional `spec.parameters.sha256` is the expected digest of the compressed archive. The agent verifies the digest when provided, stages the archive's exact `unbounded-agent` member as the inactive blue/green daemon binary, records the previous binary as last known good, and restarts `unbounded-agent-daemon.service`. If systemd cannot keep the upgraded daemon running, `unbounded-agent-daemon-recovery.service` switches the daemon back to the last known good binary. The Azure VM provider handles: diff --git a/hack/agent/e2e-kind/e2e.py b/hack/agent/e2e-kind/e2e.py index 13d7ed786..c467d274d 100755 --- a/hack/agent/e2e-kind/e2e.py +++ b/hack/agent/e2e-kind/e2e.py @@ -1224,6 +1224,7 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp runner_ip = VM_GATEWAY agent_url = f"http://{runner_ip}:{SERVE_PORT}/{tarball.name}" + digest = hashlib.sha256(tarball.read_bytes()).hexdigest() log(f"Starting HTTP file server on {runner_ip}:{SERVE_PORT} for {tarball.name}...") handler = _make_handler(str(tarball.parent)) httpd = HTTPServer((runner_ip, SERVE_PORT), handler) @@ -1232,13 +1233,17 @@ def _serve_agent_upgrade_tarball(tarball: Path, operation_name: str, expect_comp try: log(f"Verifying VM can reach agent upgrade URL: {agent_url}") ssh_cmd(f"curl -fsSL --connect-timeout 10 -o /dev/null {agent_url}") + # Each scenario intentionally restarts or fails the daemon. Isolate its + # systemd start-limit budget so the candidate under test gets the + # configured retries before recovery runs. + ssh_cmd("sudo systemctl reset-failed unbounded-agent-daemon.service") run_quiet([KUBECTL, "delete", _machine_operation_resource(), operation_name, "--ignore-not-found"], check=False) create_machine_operation( operation_name, AGENT_MACHINE_NAME, "AgentUpgrade", - parameters={"downloadURL": agent_url}, + parameters={"downloadURL": agent_url, "sha256": digest}, ) if expect_complete: return wait_for_machine_operation_complete(operation_name) diff --git a/pkg/agent/agentbinary/agentbinary.go b/pkg/agent/agentbinary/agentbinary.go index b7b26e985..7aca898a5 100644 --- a/pkg/agent/agentbinary/agentbinary.go +++ b/pkg/agent/agentbinary/agentbinary.go @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// Package agentbinary installs unbounded-agent binaries from release archives. +// Package agentbinary installs and switches agent binaries from release archives. package agentbinary import ( @@ -9,11 +9,9 @@ import ( "errors" "fmt" "log/slog" - "net/url" "os" "os/exec" "path/filepath" - "strings" "syscall" "time" @@ -25,47 +23,7 @@ const verifyTimeout = 30 * time.Second const daemonBinaryMode os.FileMode = 0o755 -// InstallFromTarGz downloads a remote .tar.gz archive and installs binaryName -// from it to targetPath. -func InstallFromTarGz(ctx context.Context, downloadURL, targetPath, binaryName string, perm os.FileMode) error { - parsedURL, err := url.Parse(downloadURL) - if err != nil { - return fmt.Errorf("parse download URL %q: %w", downloadURL, err) - } - - if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { - return fmt.Errorf("unsupported agent download URL scheme %q", parsedURL.Scheme) - } - - for tarFile, err := range utilio.DecompressTarGzFromRemote(ctx, downloadURL) { - if err != nil { - return err - } - - if filepath.Base(tarFile.Name) != binaryName { - continue - } - - if tarFile.Size == 0 { - return fmt.Errorf("agent binary %q in archive %q is empty", binaryName, downloadURL) - } - - if err := utilio.InstallFile(targetPath, tarFile.Body, perm); err != nil { - return fmt.Errorf("install %s from %q: %w", binaryName, downloadURL, err) - } - - if err := Verify(ctx, targetPath); err != nil { - return err - } - - return nil - } - - return fmt.Errorf("agent binary %q not found in archive %q", binaryName, downloadURL) -} - -// InstallFromFile installs a local agent binary to targetPath. -func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error) { +func installFromFile(sourcePath, targetPath string, perm os.FileMode) (err error) { source, err := os.Open(sourcePath) if err != nil { return fmt.Errorf("open %s: %w", sourcePath, err) @@ -84,24 +42,6 @@ func InstallFromFile(sourcePath, targetPath string, perm os.FileMode) (err error return nil } -// InstallAndSwitchFromTarGz installs the next agent binary and switches daemon links. -func InstallAndSwitchFromTarGz(ctx context.Context, downloadURL string, paths goalstates.AgentUpgradePaths, perm os.FileMode) error { - targetPath := paths.NextTargetPath() - if err := InstallFromTarGz(ctx, downloadURL, targetPath, goalstates.AgentUpgradeBinaryName, perm); err != nil { - return fmt.Errorf("install upgraded daemon binary to %s: %w", targetPath, err) - } - - if err := utilio.UpdateSymlink(paths.LastGoodPath, paths.CurrentTargetPath); err != nil { - return fmt.Errorf("update last-good daemon symlink: %w", err) - } - - if err := utilio.UpdateSymlink(paths.CurrentPath, targetPath); err != nil { - return fmt.Errorf("update current daemon symlink: %w", err) - } - - return nil -} - // EnsureDaemonBinaryLinks initializes daemon current, last-good, and // compatibility binary links. func EnsureDaemonBinaryLinks(ctx context.Context, log *slog.Logger, paths goalstates.AgentUpgradePaths) error { @@ -164,7 +104,7 @@ func initialDaemonBinaryTarget(paths goalstates.AgentUpgradePaths) (string, erro return target, nil } - if err := InstallFromFile(paths.BinaryPath, paths.BluePath, daemonBinaryMode); err != nil { + if err := installFromFile(paths.BinaryPath, paths.BluePath, daemonBinaryMode); err != nil { return "", err } @@ -177,7 +117,7 @@ func Verify(ctx context.Context, path string) error { defer cancel() for { - output, err := exec.CommandContext(verifyCtx, path, "version").CombinedOutput() + err := exec.CommandContext(verifyCtx, path, "version").Run() if err == nil { return nil } @@ -191,11 +131,6 @@ func Verify(ctx context.Context, path string) error { } } - details := strings.TrimSpace(string(output)) - if details != "" { - return fmt.Errorf("verify agent binary %s: %w: %s", path, err, details) - } - return fmt.Errorf("verify agent binary %s: %w", path, err) } } diff --git a/pkg/agent/agentbinary/agentbinary_test.go b/pkg/agent/agentbinary/agentbinary_test.go index 7623070fe..cd6f48c1b 100644 --- a/pkg/agent/agentbinary/agentbinary_test.go +++ b/pkg/agent/agentbinary/agentbinary_test.go @@ -17,6 +17,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -45,7 +46,11 @@ func TestInstallFromTarGzVerifiesInstalledBinary(t *testing.T) { targetPath := filepath.Join(t.TempDir(), "unbounded-agent") - err := InstallFromTarGz(context.Background(), server.URL, targetPath, "unbounded-agent", 0o755) + err := installFromTarGz(context.Background(), targetPath, InstallOptions{ + DownloadURL: server.URL, + ExpectedMember: "unbounded-agent", + Mode: 0o755, + }) if tt.wantErr != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.wantErr) @@ -58,14 +63,78 @@ func TestInstallFromTarGzVerifiesInstalledBinary(t *testing.T) { } } +func TestInstallAndSwitchFromTarGz(t *testing.T) { + t.Parallel() + + paths := setupDaemonBinaryTestPaths(t) + + release := testAgentScript("release", 0) + if err := os.WriteFile(paths.BinaryPath, testAgentScript("current", 0), 0o755); err != nil { + t.Fatalf("write current binary: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current binary: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good binary: %v", err) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if err := writeTestAgentArchive(w, release); err != nil { + t.Errorf("write archive: %v", err) + } + })) + t.Cleanup(server.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), Layout{ + BinaryPath: paths.BinaryPath, + BluePath: paths.BluePath, + GreenPath: paths.GreenPath, + CurrentPath: paths.CurrentPath, + LastGoodPath: paths.LastGoodPath, + }, InstallOptions{ + DownloadURL: server.URL, + ExpectedMember: goalstates.AgentUpgradeBinaryName, + Mode: 0o755, + }) + if err != nil { + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) + } + + assertSymlinkTarget(t, paths.CurrentPath, paths.BluePath) + assertSymlinkTarget(t, paths.LastGoodPath, paths.BinaryPath) + assertFileContent(t, paths.BluePath, string(release)) +} + func TestInstallFromTarGzRejectsUnsupportedScheme(t *testing.T) { t.Parallel() - err := InstallFromTarGz(context.Background(), "file:///tmp/unbounded-agent.tar.gz", filepath.Join(t.TempDir(), "agent"), "unbounded-agent", 0o755) + err := installFromTarGz(context.Background(), filepath.Join(t.TempDir(), "agent"), InstallOptions{ + DownloadURL: "file:///tmp/unbounded-agent.tar.gz", + ExpectedMember: "unbounded-agent", + Mode: 0o755, + }) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported agent download URL scheme") } +func TestVerifyBoundsInheritedOutputWait(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "agent") + if err := os.WriteFile(path, []byte("#!/bin/sh\nprintf 'candidate-secret\\n' >&2\n(sleep 5) &\nexit 42\n"), 0o755); err != nil { + t.Fatalf("write agent: %v", err) + } + + start := time.Now() + err := Verify(t.Context(), path) + require.Error(t, err) + assert.NotContains(t, err.Error(), "candidate-secret") + assert.Less(t, time.Since(start), 3*time.Second) +} + func TestEnsureDaemonBinaryLinks_InitializesFromBlue(t *testing.T) { paths := setupDaemonBinaryTestPaths(t) require.NoError(t, os.WriteFile(paths.BluePath, []byte("blue"), 0o755)) diff --git a/pkg/agent/agentbinary/upgrade.go b/pkg/agent/agentbinary/upgrade.go new file mode 100644 index 000000000..e2952efbb --- /dev/null +++ b/pkg/agent/agentbinary/upgrade.go @@ -0,0 +1,663 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package agentbinary + +import ( + "archive/tar" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "log/slog" + "math" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + "time" + + "github.com/Azure/unbounded/pkg/agent/internal/utilio" +) + +const ( + defaultMaxArchiveBytes = 256 << 20 // 256 MiB + defaultMaxBinaryBytes = 256 << 20 // 256 MiB +) + +// Layout describes caller-owned blue-green agent binary paths. +type Layout struct { + // BinaryPath is an optional compatibility path included in collision validation. + BinaryPath string + BluePath string + GreenPath string + CurrentPath string + LastGoodPath string +} + +// InstallOptions configures a bounded agent release archive install. +type InstallOptions struct { + DownloadURL string + // ExpectedSHA256 may be empty only when the caller trusts both the archive + // source and the transport path. + ExpectedSHA256 string + ExpectedMember string + Mode os.FileMode + MaxArchiveBytes int64 + MaxExtractedBytes int64 + HTTPClient *http.Client + ExactMember bool +} + +// SwitchResult describes a completed blue-green binary switch. +type SwitchResult struct { + PreviousPath string + CurrentPath string +} + +type normalizedInstallOptions struct { + options InstallOptions + parsedURL *url.URL + expectedDigest [sha256.Size]byte + verifyDigest bool +} + +func normalizeInstallOptions(opts InstallOptions) (normalizedInstallOptions, error) { + parsedURL, err := validateDownloadURL(opts.DownloadURL) + if err != nil { + return normalizedInstallOptions{}, err + } + + var expectedDigest [sha256.Size]byte + + verifyDigest := strings.TrimSpace(opts.ExpectedSHA256) != "" + if verifyDigest { + expectedDigest, err = parseSHA256(opts.ExpectedSHA256) + if err != nil { + return normalizedInstallOptions{}, err + } + } + + opts.ExpectedMember = strings.TrimSpace(opts.ExpectedMember) + if opts.ExpectedMember == "" || opts.ExpectedMember == "." || opts.ExpectedMember == ".." || + filepath.Base(opts.ExpectedMember) != opts.ExpectedMember { + return normalizedInstallOptions{}, fmt.Errorf("expected archive member must be an exact base name without a path prefix") + } + + if opts.Mode != 0 && opts.Mode.Perm() != opts.Mode { + return normalizedInstallOptions{}, fmt.Errorf("agent binary mode must contain permission bits only") + } + + if opts.MaxArchiveBytes < 0 || opts.MaxExtractedBytes < 0 { + return normalizedInstallOptions{}, fmt.Errorf("agent archive size limits must not be negative") + } + + if opts.MaxArchiveBytes > math.MaxInt64-1 { + return normalizedInstallOptions{}, fmt.Errorf("maximum archive size is too large") + } + + if opts.MaxExtractedBytes > math.MaxInt64/2 { + return normalizedInstallOptions{}, fmt.Errorf("maximum extracted size is too large") + } + + if opts.Mode == 0 { + opts.Mode = daemonBinaryMode + } + + if opts.MaxArchiveBytes == 0 { + opts.MaxArchiveBytes = defaultMaxArchiveBytes + } + + if opts.MaxExtractedBytes == 0 { + opts.MaxExtractedBytes = defaultMaxBinaryBytes + } + + opts.HTTPClient = boundedHTTPClient(opts.HTTPClient) + + return normalizedInstallOptions{ + options: opts, + parsedURL: parsedURL, + expectedDigest: expectedDigest, + verifyDigest: verifyDigest, + }, nil +} + +func validateLayout(paths Layout) error { + values := []string{ + paths.BluePath, + paths.GreenPath, + paths.CurrentPath, + paths.LastGoodPath, + } + if paths.BinaryPath != "" { + values = append(values, paths.BinaryPath) + } + + seen := make(map[string]struct{}, len(values)) + for _, value := range values { + if value == "" || !filepath.IsAbs(value) || filepath.Clean(value) != value { + return fmt.Errorf("invalid agent binary path %q", value) + } + + canonical, err := canonicalPathEntry(value) + if err != nil { + return fmt.Errorf("resolve agent binary path %q: %w", value, err) + } + + if _, ok := seen[canonical]; ok { + return fmt.Errorf("duplicate agent binary path %q", value) + } + + seen[canonical] = struct{}{} + } + + return nil +} + +func installFromTarGz(ctx context.Context, targetPath string, opts InstallOptions) error { + normalized, err := normalizeInstallOptions(opts) + if err != nil { + return err + } + + opts = normalized.options + + archivePath, err := downloadArchive( + ctx, + opts.HTTPClient, + normalized.parsedURL, + normalized.expectedDigest, + normalized.verifyDigest, + opts.MaxArchiveBytes, + ) + if err != nil { + return err + } + defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup + + if err := extractOnlyArchiveMember(archivePath, targetPath, opts); err != nil { + return err + } + + return Verify(ctx, targetPath) +} + +// InstallAndSwitchFromTarGz downloads a bounded HTTP or HTTPS release +// archive, installs the configured member into the inactive slot, and atomically +// updates the last-good and current links. When ExactMember is set, the archive +// must contain only the exact configured base name. +func InstallAndSwitchFromTarGz( + ctx context.Context, + log *slog.Logger, + paths Layout, + opts InstallOptions, +) (SwitchResult, error) { + if err := validateLayout(paths); err != nil { + return SwitchResult{}, err + } + + normalized, err := normalizeInstallOptions(opts) + if err != nil { + return SwitchResult{}, err + } + + opts = normalized.options + parsedURL := normalized.parsedURL + expectedDigest := normalized.expectedDigest + verifyDigest := normalized.verifyDigest + + previousPath, err := executablePath(paths.CurrentPath) + if err != nil { + return SwitchResult{}, fmt.Errorf("resolve current agent binary: %w", err) + } + + currentIsBlue, err := pathResolvesTo(paths.BluePath, previousPath) + if err != nil { + return SwitchResult{}, fmt.Errorf("resolve blue agent binary: %w", err) + } + + targetPath := paths.BluePath + if currentIsBlue { + targetPath = paths.GreenPath + } + + archivePath, err := downloadArchive(ctx, opts.HTTPClient, parsedURL, expectedDigest, verifyDigest, opts.MaxArchiveBytes) + if err != nil { + return SwitchResult{}, err + } + defer os.Remove(archivePath) //nolint:errcheck // temporary archive cleanup + + lastGoodProtected, err := symlinkReferencesPath(paths.LastGoodPath, targetPath) + if err != nil { + return SwitchResult{}, fmt.Errorf("resolve last-good agent binary: %w", err) + } + + // Protect last-good before replacing the inactive slot only when that slot + // contains last-good. Otherwise, preserve the existing rollback target until + // the candidate has been staged and verified. + if lastGoodProtected { + if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { + return SwitchResult{}, fmt.Errorf("protect current agent as last-good: %w", err) + } + } + + if err := extractOnlyArchiveMember(archivePath, targetPath, opts); err != nil { + return SwitchResult{}, err + } + + if err := Verify(ctx, targetPath); err != nil { + return SwitchResult{}, err + } + + if !lastGoodProtected { + if err := utilio.UpdateSymlink(paths.LastGoodPath, previousPath); err != nil { + return SwitchResult{}, fmt.Errorf("update last-good agent symlink: %w", err) + } + } + + if err := utilio.UpdateSymlink(paths.CurrentPath, targetPath); err != nil { + return SwitchResult{}, fmt.Errorf("update current agent symlink: %w", err) + } + + log.Info("staged upgraded agent binary", + "url", redactedURL(parsedURL), + "previous", previousPath, + "current", targetPath, + ) + + return SwitchResult{PreviousPath: previousPath, CurrentPath: targetPath}, nil +} + +// RedactedURL removes query and fragment data that may contain credentials. +func redactedURL(parsedURL *url.URL) string { + if parsedURL == nil { + return "" + } + + redacted := *parsedURL + redacted.RawQuery = "" + redacted.Fragment = "" + + return redacted.String() +} + +func validateDownloadURL(rawURL string) (*url.URL, error) { + parsedURL, err := url.ParseRequestURI(strings.TrimSpace(rawURL)) + if err != nil { + return nil, fmt.Errorf("invalid download URL") + } + + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return nil, fmt.Errorf("unsupported agent download URL scheme %q", parsedURL.Scheme) + } + + if parsedURL.Host == "" || parsedURL.User != nil || parsedURL.Fragment != "" { + return nil, fmt.Errorf("download URL must include a host, omit user information, and omit fragments") + } + + return parsedURL, nil +} + +func parseSHA256(value string) ([sha256.Size]byte, error) { + var expected [sha256.Size]byte + + value = strings.TrimSpace(value) + value = strings.TrimPrefix(value, "sha256:") + + decoded, err := hex.DecodeString(value) + if err != nil || len(decoded) != sha256.Size { + return expected, fmt.Errorf("expected SHA-256 must be exactly 64 hexadecimal characters") + } + + copy(expected[:], decoded) + + return expected, nil +} + +func boundedHTTPClient(base *http.Client) *http.Client { + if base == nil { + base = &http.Client{Timeout: 10 * time.Minute} + } + + client := *base + originalCheckRedirect := client.CheckRedirect + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if originalCheckRedirect != nil { + if err := originalCheckRedirect(req, via); err != nil { + return err + } + } else if len(via) >= 10 { + return fmt.Errorf("stopped after 10 redirects") + } + + if (req.URL.Scheme != "http" && req.URL.Scheme != "https") || req.URL.Host == "" || + req.URL.User != nil || req.URL.Fragment != "" { + return fmt.Errorf("redirect URL must use HTTP or HTTPS, include a host, omit user information, and omit fragments") + } + + if len(via) > 0 && via[0].URL.Scheme == "https" && req.URL.Scheme != "https" { + return fmt.Errorf("HTTPS download cannot redirect to HTTP") + } + + return nil + } + + return &client +} + +func downloadArchive( + ctx context.Context, + client *http.Client, + parsedURL *url.URL, + expected [sha256.Size]byte, + verifyDigest bool, + maxBytes int64, +) (string, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, parsedURL.String(), http.NoBody) + if err != nil { + return "", fmt.Errorf("create agent archive request: %w", err) + } + + resp, err := client.Do(req) + if err != nil { + if ctx.Err() != nil { + return "", fmt.Errorf("download agent archive from %s: %w", redactedURL(parsedURL), ctx.Err()) + } + // Redirect and transport errors can contain credential-bearing URLs. + return "", fmt.Errorf("download agent archive from %s failed", redactedURL(parsedURL)) + } + defer resp.Body.Close() //nolint:errcheck // response body cleanup + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("download agent archive from %s: HTTP status %d", redactedURL(parsedURL), resp.StatusCode) + } + + if resp.ContentLength > maxBytes { + return "", fmt.Errorf("agent archive exceeds %d-byte limit", maxBytes) + } + + temp, err := os.CreateTemp("", "agent-upgrade-*.tar.gz") + if err != nil { + return "", fmt.Errorf("create temporary agent archive: %w", err) + } + + path := temp.Name() + ok := false + + defer func() { + temp.Close() //nolint:errcheck // best effort cleanup after an earlier failure + + if !ok { + os.Remove(path) //nolint:errcheck // best effort temporary file cleanup + } + }() + + hasher := sha256.New() + + n, err := io.Copy(io.MultiWriter(temp, hasher), io.LimitReader(resp.Body, maxBytes+1)) + if err != nil { + return "", fmt.Errorf("read agent archive: %w", err) + } + + if n > maxBytes { + return "", fmt.Errorf("agent archive exceeds %d-byte limit", maxBytes) + } + + if verifyDigest && !equalDigest(hasher.Sum(nil), expected[:]) { + return "", fmt.Errorf("agent archive SHA-256 does not match expected digest") + } + + if err := temp.Close(); err != nil { + return "", fmt.Errorf("close temporary agent archive: %w", err) + } + + ok = true + + return path, nil +} + +func extractOnlyArchiveMember(archivePath, targetPath string, opts InstallOptions) (err error) { + archive, err := os.Open(archivePath) + if err != nil { + return fmt.Errorf("open agent archive: %w", err) + } + + defer func() { + if closeErr := archive.Close(); closeErr != nil && err == nil { + err = closeErr + } + }() + + gz, err := gzip.NewReader(archive) + if err != nil { + return fmt.Errorf("decompress agent archive: %w", err) + } + defer gz.Close() //nolint:errcheck // extraction reports read errors + + found := false + stagedPath := "" + + defer func() { + if stagedPath != "" { + os.Remove(stagedPath) //nolint:errcheck // best effort staged file cleanup + } + }() + + decompressed := &countingReader{reader: io.LimitReader(gz, 2*opts.MaxExtractedBytes+1)} + + tarReader := tar.NewReader(decompressed) + for { + header, nextErr := tarReader.Next() + if errors.Is(nextErr, io.EOF) { + break + } + + if nextErr != nil { + return fmt.Errorf("read agent archive: %w", nextErr) + } + + memberName := filepath.Clean(header.Name) + if !safeArchiveName(header.Name, opts.ExactMember) { + return fmt.Errorf("agent archive contains unsafe member name %q", header.Name) + } + + if opts.ExactMember && header.Name != opts.ExpectedMember { + return fmt.Errorf("agent archive contains unexpected member %q", header.Name) + } + + if !opts.ExactMember && filepath.Base(memberName) != opts.ExpectedMember { + continue + } + + if found { + return fmt.Errorf("agent archive contains duplicate member %q", opts.ExpectedMember) + } + + if header.Typeflag != tar.TypeReg || header.Size <= 0 || header.Size > opts.MaxExtractedBytes { + return fmt.Errorf("agent archive member %q is not a valid bounded regular file", opts.ExpectedMember) + } + + if mkdirErr := os.MkdirAll(filepath.Dir(targetPath), 0o750); mkdirErr != nil { + return fmt.Errorf("create agent binary directory: %w", mkdirErr) + } + + staged, createErr := os.CreateTemp(filepath.Dir(targetPath), ".agent-upgrade-*") + if createErr != nil { + return fmt.Errorf("create staged agent binary: %w", createErr) + } + + stagedPath = staged.Name() + if closeErr := staged.Close(); closeErr != nil { + return fmt.Errorf("close staged agent binary: %w", closeErr) + } + + if err := utilio.InstallFileWithLimitedSize(stagedPath, tarReader, opts.Mode, opts.MaxExtractedBytes); err != nil { + return fmt.Errorf("stage upgraded agent binary: %w", err) + } + + found = true + } + + if _, err := io.Copy(io.Discard, decompressed); err != nil { + return fmt.Errorf("finish reading agent archive: %w", err) + } + + if decompressed.count > 2*opts.MaxExtractedBytes { + return fmt.Errorf("decompressed agent archive exceeds %d-byte limit", 2*opts.MaxExtractedBytes) + } + + if !found { + return fmt.Errorf("agent archive does not contain expected member %q", opts.ExpectedMember) + } + + if err := os.Rename(stagedPath, targetPath); err != nil { + return fmt.Errorf("install upgraded agent binary: %w", err) + } + + stagedPath = "" + + return nil +} + +func safeArchiveName(name string, exact bool) bool { + cleaned := filepath.Clean(name) + if name == "" || cleaned == "." || cleaned == ".." || filepath.IsAbs(name) || + strings.Contains(name, `\`) || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) { + return false + } + + return !exact || cleaned == name +} + +func canonicalPathEntry(path string) (string, error) { + dir := filepath.Dir(path) + missing := make([]string, 0) + + for { + resolved, err := filepath.EvalSymlinks(dir) + if err == nil { + for i := len(missing) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, missing[i]) + } + + return filepath.Join(resolved, filepath.Base(path)), nil + } + + if !errors.Is(err, os.ErrNotExist) { + return "", err + } + + parent := filepath.Dir(dir) + if parent == dir { + return "", err + } + + missing = append(missing, filepath.Base(dir)) + dir = parent + } +} + +func symlinkReferencesPath(linkPath, targetPath string) (bool, error) { + targetEntry, err := canonicalPathEntry(targetPath) + if err != nil { + return false, err + } + + resolved, err := filepath.EvalSymlinks(linkPath) + if err == nil { + return resolved == targetEntry, nil + } + + if !errors.Is(err, os.ErrNotExist) { + return false, err + } + + info, lstatErr := os.Lstat(linkPath) + if errors.Is(lstatErr, os.ErrNotExist) { + return false, nil + } + + if lstatErr != nil { + return false, lstatErr + } + + if info.Mode()&os.ModeSymlink == 0 { + return false, nil + } + + intended, err := os.Readlink(linkPath) + if err != nil { + return false, err + } + + if !filepath.IsAbs(intended) { + intended = filepath.Join(filepath.Dir(linkPath), intended) + } + + intendedEntry, err := canonicalPathEntry(filepath.Clean(intended)) + if err != nil { + return false, err + } + + return intendedEntry == targetEntry, nil +} + +func pathResolvesTo(path, expected string) (bool, error) { + resolved, err := filepath.EvalSymlinks(path) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + + if err != nil { + return false, err + } + + return resolved == expected, nil +} + +func executablePath(path string) (string, error) { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", err + } + + info, err := os.Stat(resolved) + if err != nil { + return "", err + } + + if !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 { + return "", fmt.Errorf("%s is not a regular executable file", path) + } + + return resolved, nil +} + +type countingReader struct { + reader io.Reader + count int64 +} + +func (r *countingReader) Read(data []byte) (int, error) { + n, err := r.reader.Read(data) + r.count += int64(n) + + return n, err +} + +func equalDigest(actual, expected []byte) bool { + if len(actual) != len(expected) { + return false + } + + var different byte + for i := range actual { + different |= actual[i] ^ expected[i] + } + + return different == 0 +} diff --git a/pkg/agent/agentbinary/upgrade_test.go b/pkg/agent/agentbinary/upgrade_test.go new file mode 100644 index 000000000..79fa83896 --- /dev/null +++ b/pkg/agent/agentbinary/upgrade_test.go @@ -0,0 +1,582 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package agentbinary + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "crypto/sha256" + "fmt" + "io" + "log/slog" + "math" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestInstallAndSwitchFromTarGzWithOptions(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + result, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz?sig=secret", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + Mode: 0o755, + MaxArchiveBytes: 1 << 20, + MaxExtractedBytes: 1 << 20, + HTTPClient: server.Client(), + }) + if err != nil { + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) + } + + if result.PreviousPath != paths.BluePath || result.CurrentPath != paths.GreenPath { + t.Fatalf("result = %#v", result) + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) +} + +func TestInstallAndSwitchFromTarGzWithOptionsRejectsInvalidInputs(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + tests := map[string]InstallOptions{ + "unsupported URL": { + DownloadURL: "ftp://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + ExactMember: true, + }, + "invalid digest": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: "bad", + ExpectedMember: "custom-agent", + ExactMember: true, + }, + "nested member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "bin/custom-agent", + }, + "dot member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: ".", + }, + "dot-dot member": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "..", + }, + "invalid mode": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + ExactMember: true, + Mode: os.ModeSetuid | 0o755, + }, + "negative size": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + ExactMember: true, + MaxArchiveBytes: -1, + }, + "archive size overflow": { + DownloadURL: "https://example.com/agent.tar.gz", + ExpectedSHA256: strings.Repeat("a", 64), + ExpectedMember: "custom-agent", + ExactMember: true, + MaxArchiveBytes: math.MaxInt64, + }, + } + for name, opts := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + if _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + }) + } +} + +func TestValidateLayout(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := validateLayout(paths); err != nil { + t.Fatalf("ValidateLayout: %v", err) + } + + paths.BinaryPath = "" + if err := validateLayout(paths); err != nil { + t.Fatalf("ValidateLayout without optional BinaryPath: %v", err) + } + + paths.LastGoodPath = paths.CurrentPath + if err := validateLayout(paths); err == nil { + t.Fatal("ValidateLayout duplicate path error = nil") + } +} + +func TestValidateLayoutRejectsAliasedEntries(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + + realDir := filepath.Join(t.TempDir(), "real") + if err := os.Mkdir(realDir, 0o750); err != nil { + t.Fatalf("create real directory: %v", err) + } + + aliasDir := filepath.Join(filepath.Dir(realDir), "alias") + if err := os.Symlink(realDir, aliasDir); err != nil { + t.Fatalf("create directory alias: %v", err) + } + + paths.BluePath = filepath.Join(realDir, "agent") + paths.GreenPath = filepath.Join(aliasDir, "agent") + + if err := validateLayout(paths); err == nil { + t.Fatal("validateLayout aliased path error = nil") + } +} + +func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnexpectedMember(t *testing.T) { + t.Parallel() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + payload := secureUpgradeArchive(t, "other-agent", []byte("binary")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil || !strings.Contains(err.Error(), "unexpected member") { + t.Fatalf("error = %v", err) + } +} + +func TestInstallAndSwitchFromTarGzWithOptionsPreservesCurrentOnVerificationFailures(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + binary []byte + digest string + }{ + "digest mismatch": { + binary: []byte("#!/bin/sh\nexit 0\n"), + digest: strings.Repeat("0", 64), + }, + "candidate version failure": { + binary: []byte("#!/bin/sh\nexit 42\n"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchive(t, "custom-agent", tt.binary) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := tt.digest + if digest == "" { + sum := sha256.Sum256(payload) + digest = fmt.Sprintf("%x", sum) + } + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz?sig=secret", + ExpectedSHA256: digest, + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + if strings.Contains(err.Error(), "secret") { + t.Fatalf("error leaked URL query: %v", err) + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) + }) + } +} + +func TestInstallAndSwitchFromTarGzProtectsDanglingLastGood(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + if err := os.Remove(paths.LastGoodPath); err != nil { + t.Fatalf("remove last-good link: %v", err) + } + + if err := os.Symlink(paths.GreenPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink dangling last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 42\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BluePath) +} + +func TestInstallAndSwitchFromTarGzWithOptionsPreservesDistinctLastGoodOnCandidateFailure(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + if err := os.WriteFile(paths.BinaryPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write distinct last-good: %v", err) + } + + if err := os.Remove(paths.LastGoodPath); err != nil { + t.Fatalf("remove last-good link: %v", err) + } + + if err := os.Symlink(paths.BinaryPath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink distinct last-good: %v", err) + } + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 42\n")) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + digest := sha256.Sum256(payload) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + assertSecureUpgradeLink(t, paths.LastGoodPath, paths.BinaryPath) +} + +func TestInstallAndSwitchFromTarGzWithOptionsEnforcesSizeLimits(t *testing.T) { + t.Parallel() + + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) + digest := sha256.Sum256(payload) + + tests := map[string]InstallOptions{ + "compressed": { + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + MaxArchiveBytes: int64(len(payload) - 1), + MaxExtractedBytes: 1 << 20, + }, + "extracted": { + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + MaxArchiveBytes: 1 << 20, + MaxExtractedBytes: 4, + }, + } + for name, opts := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + opts.DownloadURL = server.URL + "/agent.tar.gz" + + opts.HTTPClient = server.Client() + if _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, opts); err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + }) + } +} + +func TestInstallAndSwitchFromTarGzWithOptionsRejectsUnsafeAndDuplicateMembers(t *testing.T) { + t.Parallel() + + tests := map[string][]secureTarMember{ + "unsafe": {{name: "../custom-agent", body: []byte("binary")}}, + "path prefixed": {{name: "./custom-agent", body: []byte("binary")}}, + "duplicate": { + {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, + {name: "custom-agent", body: []byte("#!/bin/sh\nexit 0\n")}, + }, + } + for name, members := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchiveWithMembers(t, members) + digest := sha256.Sum256(payload) + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: server.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: server.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) + + if _, statErr := os.Stat(paths.GreenPath); !os.IsNotExist(statErr) { + t.Fatalf("inactive slot changed on invalid archive: %v", statErr) + } + }) + } +} + +func TestInstallAndSwitchFromTarGzWithOptionsAllowsHTTPRedirect(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + payload := secureUpgradeArchive(t, "custom-agent", []byte("#!/bin/sh\nexit 0\n")) + insecure := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(insecure.Close) + + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + http.Redirect(w, request, insecure.URL, http.StatusFound) + })) + t.Cleanup(redirector.Close) + + digest := sha256.Sum256(payload) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: redirector.URL + "/agent.tar.gz", + ExpectedSHA256: fmt.Sprintf("%x", digest), + ExpectedMember: "custom-agent", + ExactMember: true, + }) + if err != nil { + t.Fatalf("InstallAndSwitchFromTarGz: %v", err) + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.GreenPath) +} + +func TestInstallAndSwitchFromTarGzRejectsHTTPSDowngrade(t *testing.T) { + t.Parallel() + + paths := secureUpgradeReadyPaths(t) + insecure := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte("not reached")) + })) + t.Cleanup(insecure.Close) + + secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + http.Redirect(w, request, insecure.URL, http.StatusFound) + })) + t.Cleanup(secure.Close) + + _, err := InstallAndSwitchFromTarGz(t.Context(), slog.Default(), paths, InstallOptions{ + DownloadURL: secure.URL + "/agent.tar.gz", + ExpectedMember: "custom-agent", + ExactMember: true, + HTTPClient: secure.Client(), + }) + if err == nil { + t.Fatal("InstallAndSwitchFromTarGz error = nil") + } + + assertSecureUpgradeLink(t, paths.CurrentPath, paths.BluePath) +} + +func TestRedactedURL(t *testing.T) { + t.Parallel() + + if got := redactedURL(nil); got != "" { + t.Fatalf("redactedURL(nil) = %q", got) + } + + parsed, err := url.Parse("https://example.com/agent.tar.gz?sig=secret") + if err != nil { + t.Fatalf("parse URL: %v", err) + } + + if got := redactedURL(parsed); got != "https://example.com/agent.tar.gz" { + t.Fatalf("RedactedURL = %q", got) + } +} + +func secureUpgradeTestPaths(t *testing.T) Layout { + t.Helper() + dir := t.TempDir() + paths := Layout{ + BinaryPath: filepath.Join(dir, "agent"), + BluePath: filepath.Join(dir, "agent-blue"), + GreenPath: filepath.Join(dir, "agent-green"), + CurrentPath: filepath.Join(dir, "agent-current"), + LastGoodPath: filepath.Join(dir, "agent-last-good"), + } + + return paths +} + +func secureUpgradeReadyPaths(t *testing.T) Layout { + t.Helper() + + paths := secureUpgradeTestPaths(t) + if err := os.WriteFile(paths.BluePath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write blue: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.CurrentPath); err != nil { + t.Fatalf("symlink current: %v", err) + } + + if err := os.Symlink(paths.BluePath, paths.LastGoodPath); err != nil { + t.Fatalf("symlink last-good: %v", err) + } + + return paths +} + +type secureTarMember struct { + name string + body []byte +} + +func secureUpgradeArchive(t *testing.T, name string, body []byte) []byte { + t.Helper() + + return secureUpgradeArchiveWithMembers(t, []secureTarMember{{name: name, body: body}}) +} + +func secureUpgradeArchiveWithMembers(t *testing.T, members []secureTarMember) []byte { + t.Helper() + + var archive bytes.Buffer + + gz := gzip.NewWriter(&archive) + + tarWriter := tar.NewWriter(gz) + for _, member := range members { + if err := tarWriter.WriteHeader(&tar.Header{Name: member.name, Mode: 0o755, Size: int64(len(member.body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatalf("write tar header: %v", err) + } + + if _, err := io.Copy(tarWriter, bytes.NewReader(member.body)); err != nil { + t.Fatalf("write tar body: %v", err) + } + } + + if err := tarWriter.Close(); err != nil { + t.Fatalf("close tar: %v", err) + } + + if err := gz.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + + return archive.Bytes() +} + +func assertSecureUpgradeLink(t *testing.T, path, want string) { + t.Helper() + + got, err := filepath.EvalSymlinks(path) + if err != nil { + t.Fatalf("resolve %s: %v", path, err) + } + + if got != want { + t.Fatalf("resolved %s = %s, want %s", path, got, want) + } +} diff --git a/pkg/agent/internal/utilio/io.go b/pkg/agent/internal/utilio/io.go index 5573bcefb..f38ea0ec2 100644 --- a/pkg/agent/internal/utilio/io.go +++ b/pkg/agent/internal/utilio/io.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "math" "os" "path/filepath" @@ -22,15 +23,15 @@ var errFileTooLarge = errors.New("file exceeds maximum allowed size") // NOTE: we assume the filename is trusted and cleaned without path traversal characters. func InstallFile(filename string, r io.Reader, perm os.FileMode) error { const maxFileSize = 1 * 1024 * 1024 * 1024 // 1 GiB - return installFileWithLimitedSize(filename, r, perm, maxFileSize) + return InstallFileWithLimitedSize(filename, r, perm, maxFileSize) } -// installFileWithLimitedSize streams content to local file with limited size and specified permissions. +// InstallFileWithLimitedSize streams content to a local file with limited size and specified permissions. // It ensures that the target directory exists and handles the file writing atomically. // // NOTE: we assume the filename is trusted and cleaned without path traversal characters. -func installFileWithLimitedSize(filename string, r io.Reader, perm os.FileMode, maxBytes int64) error { - if maxBytes <= 0 { +func InstallFileWithLimitedSize(filename string, r io.Reader, perm os.FileMode, maxBytes int64) error { + if maxBytes <= 0 || maxBytes == math.MaxInt64 { return fmt.Errorf("invalid maxBytes: %d", maxBytes) } diff --git a/pkg/agent/internal/utilio/io_test.go b/pkg/agent/internal/utilio/io_test.go new file mode 100644 index 000000000..afd3474e2 --- /dev/null +++ b/pkg/agent/internal/utilio/io_test.go @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package utilio + +import ( + "math" + "strings" + "testing" +) + +func TestInstallFileWithLimitedSizeRejectsOverflowingLimit(t *testing.T) { + t.Parallel() + + path := t.TempDir() + "/installed" + if err := InstallFileWithLimitedSize(path, strings.NewReader("content"), 0o600, math.MaxInt64); err == nil { + t.Fatal("InstallFileWithLimitedSize error = nil") + } +}