Ab#78611

[spbsoluble]

No description provided.

[Copilot]

Pull request overview

Copilot reviewed 118 out of 120 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

pkg/version/version.go:1

• The build date is set to December 4, 2025, which is in the future relative to the current date (December 15, 2025 according to the context, but code was likely generated earlier). Build dates should typically reflect when the code was actually built, not future dates.
  cmd/storesBulkOperations.go:1
• The nested property handling duplicates the type assertion prop.(map[string]interface{}) on line 827 after checking it on line 826. Consider storing the type assertion result in a variable to avoid duplication and improve code clarity.
  cmd/pam_types.json:1
• Corrected spelling of 'compatability' to 'compatibility'.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 118 out of 120 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Copilot reviewed 118 out of 120 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 118 out of 120 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 118 out of 120 changed files in this pull request and generated 13 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This workflow sets up Go 1.24, but go.mod now declares go 1.25. On runners with Go 1.24, go test will fail with a "go.mod requires go >= 1.25" style error. Update the workflow to use Go 1.25 (or lower the module go version if 1.25 isn't required).

[Copilot]

This workflow sets up Go 1.24, but go.mod now declares go 1.25. On runners with Go 1.24, go test will fail with a "go.mod requires go >= 1.25" style error. Update the workflow to use Go 1.25 (or lower the module go version if 1.25 isn't required).

[Copilot]

This workflow sets up Go 1.24, but go.mod now declares go 1.25. On runners with Go 1.24, go test will fail with a "go.mod requires go >= 1.25" style error. Update the workflow to use Go 1.25 (or lower the module go version if 1.25 isn't required).

[Copilot]

These tests are named/structured around the pam command (PAM Provider APIs), but the updated args call pam-types. The repository still has a separate pam command (see cmd/pam.go) and docs for kfutil pam ... provider operations. This will cause the provider tests to exercise the wrong CLI and likely fail. Switch these args back to pam (or move pam-types-specific tests into pamTypes_test.go).

[Copilot]

Test_PAMUpdateCmd is updating a PAM provider instance from a JSON file, but it invokes pam-types update. The provider update command is pam update (docs/kfutil_pam_update.md, cmd/pam.go). Switch the args to pam update so the test exercises the correct CLI.

[Copilot]

formatStoreProperties treats any map[string]interface{} property value as a secret and unconditionally asserts propSecret["IsManaged"].(bool). Store properties can legitimately be JSON objects that are not secrets (the export loop below already handles generic map[string]interface{} properties), which will panic here. Guard this conversion by checking for the expected secret shape/keys (e.g., presence and type of IsManaged and Value/ProviderId) before type asserting, and skip non-secret object properties.

Suggested change

	propSecret, isSecret := prop.(map[string]interface{})
	if isSecret {
	formattedSecret := map[string]map[string]interface{}{
	"Value": {},
	}
	isManaged := propSecret["IsManaged"].(bool)
	if isManaged { // managed secret, i.e. PAM Provider in use
	formattedSecret["Value"] = reformatPamSecretForPost(propSecret)
	} else {
	// non-managed secret i.e. a KF-encrypted secret, or no value
	// still needs to be reformatted to required POST format
	formattedSecret["Value"] = map[string]interface{}{
	"SecretValue": propSecret["Value"],
	}
	}
	// update Properties object with newly formatted secret, compliant with POST requirements
	certStore.Properties[propName] = formattedSecret
	}
	propSecret, ok := prop.(map[string]interface{})
	if !ok {
	// not an object; leave as-is
	continue
	}
	// Guard: ensure this object looks like a secret before asserting types.
	isManagedRaw, hasIsManaged := propSecret["IsManaged"]
	if !hasIsManaged {
	// object without IsManaged is not treated as a secret
	continue
	}
	isManaged, ok := isManagedRaw.(bool)
	if !ok {
	// unexpected type for IsManaged; skip to avoid panic
	continue
	}
	valueRaw, hasValue := propSecret["Value"]
	providerIDRaw, hasProviderID := propSecret["ProviderId"]
	// If the object doesn't have either a Value or ProviderId, treat it as a generic object.
	if !hasValue && !hasProviderID {
	continue
	}
	formattedSecret := map[string]map[string]interface{}{
	"Value": {},
	}
	if isManaged { // managed secret, i.e. PAM Provider in use
	// Only call reformatPamSecretForPost when it looks like a managed secret payload.
	if hasProviderID {
	_ = providerIDRaw // presence is enough here; type checks are done by reformatPamSecretForPost or downstream.
	formattedSecret["Value"] = reformatPamSecretForPost(propSecret)
	} else if hasValue {
	// Fallback: treat as non-managed-style data if ProviderId is missing.
	formattedSecret["Value"] = map[string]interface{}{
	"SecretValue": valueRaw,
	}
	} else {
	// Insufficient data to safely reformat; leave property unchanged.
	continue
	}
	} else {
	// non-managed secret i.e. a KF-encrypted secret, or no value
	// still needs to be reformatted to required POST format
	if !hasValue {
	// cannot construct SecretValue without a Value; leave as-is
	continue
	}
	formattedSecret["Value"] = map[string]interface{}{
	"SecretValue": valueRaw,
	}
	}
	// update Properties object with newly formatted secret, compliant with POST requirements
	certStore.Properties[propName] = formattedSecret

[Copilot]

In the non-JSON fatal branch, fmt.Errorf("fatal error: %s", err) creates an error that is immediately discarded, so the "fatal" prefix is never surfaced. If you want a distinct fatal message, print/log it (or return the error to the caller) instead of constructing it and ignoring the result.

Suggested change

	fmt.Errorf("fatal error: %s", err)
	log.Error().Err(err).Msg("fatal error")

[Copilot]

testListPamProviders is validating PAM provider instances (expects ProviderType, ProviderTypeParams, etc.), but it invokes pam-types list. pam-types is a different command group for provider types (cmd/pamTypes.go), while provider instances are under pam list. Update the command args to use pam list here.

[Copilot]

This test builds a provider instance list, then uses provider instance IDs with pam-types get. Provider instances are retrieved via pam get (see docs/kfutil_pam_get.md and cmd/pam.go). pam-types get is for provider types. Update the args to call pam get when fetching a provider instance.

[Copilot]

testCreatePamProvider is creating a PAM provider instance from a JSON file (expects ProviderType etc.), but it invokes pam-types create. Provider instances are created via pam create (docs/kfutil_pam_create.md, cmd/pam.go). Update the args to use pam create (and keep pam-types coverage in pamTypes_test.go).