[ACTP] recover from a corrupt persisted identity

A truncated or malformed identity file could keep ensure-enrollment failing
until it was deleted by hand, since persistIdentityToFile writes in place and a
crash mid-write leaves partial JSON behind.

Content-level damage is now marked with ErrIdentityCorrupt and treated as
unusable: ensure-enrollment warns, discards it, and continues to the configured
identity or self-enrollment, removing the file when a configured identity takes
over so par-control cannot pick it up again. I/O and K8s secret failures still
abort, because they may be transient and re-enrolling would register a second
runner for the same host.

Author: embeaken

cmd/privateactionrunner/subcommands/ensureenrollment/command.go

@@ -66,17 +66,25 @@ func ensureEnrollment(ctx context.Context, logger log.Component, cfg config.Comp
 		return fmt.Errorf("failed to get agent identifier: %w", err)
 	}
 
+	discardPersisted := false
+
 	persisted, err := enrollment.GetIdentityFromPreviousEnrollment(ctx, cfg)
-	if err != nil {
+	switch {
+	case err != nil && !errors.Is(err, enrollment.ErrIdentityCorrupt):
+		// May be transient; re-enrolling would register a second runner.
 		return fmt.Errorf("failed to load persisted identity: %w", err)
-	}
-	if persisted != nil {
+	case err != nil:
+		logger.Warnf("Discarding unusable persisted identity: %v", err)
+		discardPersisted = true
+	case persisted != nil:
 		if err := validateIdentity(persisted.URN, persisted.PrivateKey); err != nil {
-			return fmt.Errorf("persisted identity is invalid: %w", err)
-		}
-		if !enrollment.ShouldReenroll(agentIdentifier, persisted) {
+			logger.Warnf("Discarding invalid persisted identity: %v", err)
+			discardPersisted = true
+		} else if !enrollment.ShouldReenroll(agentIdentifier, persisted) {
 			logger.Info("Persisted identity is valid; enrollment is not required")
 			return nil
+		} else {
+			discardPersisted = true
 		}
 	}
 
@@ -86,10 +94,9 @@ func ensureEnrollment(ctx context.Context, logger log.Component, cfg config.Comp
 		return err
 	}
 	if configuredURN != "" && configuredPrivateKey != "" {
-		// Rust gives a persisted file precedence over inline configuration and does
-		// not repeat the hostname comparison, so remove the stale file before
-		// allowing it to fall back to this configured identity.
-		if persisted != nil {
+		// Rust prefers a persisted file over inline configuration and repeats none of
+		// the checks above, so the unusable file has to go.
+		if discardPersisted {
 			if err := enrollment.RemoveIdentityFile(cfg); err != nil {
 				return err
 			}

cmd/privateactionrunner/subcommands/ensureenrollment/command_test.go

@@ -72,6 +72,42 @@ func TestEnsureEnrollmentPersistedIdentity(t *testing.T) {
 	}
 }
 
+func TestEnsureEnrollmentCorruptIdentityFallsBack(t *testing.T) {
+	t.Run("self-enrolls", func(t *testing.T) {
+		identityPath := filepath.Join(t.TempDir(), "identity.json")
+		require.NoError(t, os.WriteFile(identityPath, []byte("not-json"), 0o600))
+		cfg := testConfig(t, identityPath, map[string]interface{}{
+			"private_action_runner.self_enroll": true,
+		})
+		hostnameComp, _ := hostnamemock.NewMock("test-host")
+		enrollCalls := 0
+
+		err := ensureEnrollment(context.Background(), logmock.New(t), cfg, hostnameComp, func(_ context.Context, _ log.Component, _ coreconfig.Component, _ *enrollment.AgentIdentifier) (*enrollment.Result, error) {
+			enrollCalls++
+			return &enrollment.Result{URN: validURN()}, nil
+		})
+
+		require.NoError(t, err)
+		assert.Equal(t, 1, enrollCalls)
+	})
+
+	t.Run("configured identity wins and the file is removed", func(t *testing.T) {
+		identityPath := filepath.Join(t.TempDir(), "identity.json")
+		require.NoError(t, os.WriteFile(identityPath, []byte("not-json"), 0o600))
+		cfg := testConfig(t, identityPath, map[string]interface{}{
+			"private_action_runner.self_enroll": false,
+			"private_action_runner.urn":         validURN(),
+			"private_action_runner.private_key": validPrivateKey(t),
+		})
+		hostnameComp, _ := hostnamemock.NewMock("test-host")
+
+		err := ensureEnrollment(context.Background(), logmock.New(t), cfg, hostnameComp, failIfEnrolled(t))
+
+		require.NoError(t, err)
+		assert.NoFileExists(t, identityPath)
+	})
+}
+
 func TestEnsureEnrollmentMissingIdentityEnrolls(t *testing.T) {
 	identityPath := filepath.Join(t.TempDir(), "missing.json")
 	cfg := testConfig(t, identityPath, map[string]interface{}{
@@ -152,17 +188,33 @@ func TestEnsureEnrollmentFailures(t *testing.T) {
 		assert.Contains(t, err.Error(), "private_action_runner.self_enroll is false")
 	})
 
-	t.Run("malformed persisted identity", func(t *testing.T) {
+	t.Run("corrupt persisted identity without any fallback", func(t *testing.T) {
 		identityPath := filepath.Join(t.TempDir(), "identity.json")
 		require.NoError(t, os.WriteFile(identityPath, []byte("not-json"), 0o600))
-		cfg := testConfig(t, identityPath, nil)
+		cfg := testConfig(t, identityPath, map[string]interface{}{
+			"private_action_runner.self_enroll": false,
+		})
+		hostnameComp, _ := hostnamemock.NewMock("test-host")
+
+		err := ensureEnrollment(context.Background(), logmock.New(t), cfg, hostnameComp, failIfEnrolled(t))
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "private_action_runner.self_enroll is false")
+	})
+
+	t.Run("unreadable persisted identity is not discarded", func(t *testing.T) {
+		// A directory stands in for any I/O failure.
+		identityPath := filepath.Join(t.TempDir(), "identity.json")
+		require.NoError(t, os.Mkdir(identityPath, 0o700))
+		cfg := testConfig(t, identityPath, map[string]interface{}{
+			"private_action_runner.self_enroll": true,
+		})
 		hostnameComp, _ := hostnamemock.NewMock("test-host")
 
 		err := ensureEnrollment(context.Background(), logmock.New(t), cfg, hostnameComp, failIfEnrolled(t))
 
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "failed to load persisted identity")
-		assert.Contains(t, err.Error(), "failed to parse identity file JSON")
 	})
 
 	t.Run("invalid configured private key", func(t *testing.T) {

pkg/privateactionrunner/enrollment/storage.go

@@ -21,6 +21,10 @@ import (
 	"github.com/DataDog/datadog-agent/pkg/util/flavor"
 )
 
+// ErrIdentityCorrupt marks unusable persisted identity content, such as
+// truncated JSON. Not returned for I/O failures, which may be transient.
+var ErrIdentityCorrupt = errors.New("persisted identity is corrupt")
+
 // GetIdentityFromPreviousEnrollment retrieves PAR identity from either K8s secret or file based on configuration
 func GetIdentityFromPreviousEnrollment(ctx context.Context, cfg configModel.Reader) (*PersistedIdentity, error) {
 	if cfg.GetBool(setup.PARIdentityUseK8sSecret) && flavor.GetFlavor() == flavor.ClusterAgent {
@@ -59,14 +63,14 @@ func getIdentityFromFile(cfg configModel.Reader) (*PersistedIdentity, error) {
 
 	var identityContent PersistedIdentity
 	if err := json.Unmarshal(data, &identityContent); err != nil {
-		return nil, fmt.Errorf("failed to parse identity file JSON: %w", err)
+		return nil, fmt.Errorf("%w: failed to parse identity file JSON: %w", ErrIdentityCorrupt, err)
 	}
 
 	if identityContent.URN == "" {
-		return nil, errors.New("URN is empty in identity file")
+		return nil, fmt.Errorf("%w: URN is empty in identity file", ErrIdentityCorrupt)
 	}
 	if identityContent.PrivateKey == "" {
-		return nil, errors.New("private key is empty in identity file")
+		return nil, fmt.Errorf("%w: private key is empty in identity file", ErrIdentityCorrupt)
 	}
 
 	return &identityContent, nil