Skip to content

Commit 5fc9691

Browse files
gl-johnsonGitHub Enterprise
authored andcommitted
Merge pull request #110 from Conjur-Enterprise/CNJR-12572
CNJR-12572: V2 Batch Retrieval in Secrets Provider
2 parents 8e897ff + 3dcf1b9 commit 5fc9691

8 files changed

Lines changed: 251 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1717
- Support for onboarding labeled Kubernetes secrets. (CNJR-12567)
1818
- Remove stale secrets for labeled Kubernetes secrets. (CNJR-12714)
1919
- Remove all Conjur managed secrets if conjur-map is missing or empty. (CNJR-12857)
20+
- V2 batch retrieval as the default method to fetch secrets with automatic fallback to V1 for backwards compatibility. (CNJR-12572)
2021

2122
### Fixed
2223
- Fix deadlock when using sidecar without refresh. (CNJR-12825)

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ require (
7070

7171
require (
7272
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
73-
github.com/cyberark/conjur-api-go v0.13.13 // version will be ignored by auto release process
73+
github.com/cyberark/conjur-api-go v0.13.16 // version will be ignored by auto release process
7474
github.com/cyberark/conjur-authn-k8s-client v0.26.10 // version will be ignored by auto release process
7575
github.com/cyberark/conjur-opentelemetry-tracer v1.55.55 // version will be ignored by auto release process
7676
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect

pkg/log/messages/debug_messages.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ const CSPFK013D string = "CSPFK013D Kubernetes Secret '%s' has no '%s' annotatio
1515
const CSPFK014D string = "CSPFK014D Found %d secret group templates for Kubernetes secret %s"
1616
const CSPFK015D string = "CSPFK015D No secrets to update"
1717
const CSPFK016D string = "CSPFK016D No change in Kubernetes secret '%s'"
18+
const CSPFK017D string = "CSPFK017D Using V1 batch retrieval"

pkg/log/messages/error_messages.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,9 @@ const CSPFK065E string = "CSPFK065E Secret group %s: the content-type of %s is i
9393

9494
// Fetch All
9595
const CSPFK066E string = "CSPFK066E Stopped fetching additional secrets after %d secrets were fetched"
96+
const CSPFK090E string = "CSPFK090E V2 batch retrieval not available, falling back to V1: %s"
97+
const CSPFK091E string = "CSPFK091E Some secrets failed to retrieve in V2 batch request: %s"
98+
const CSPFK092E string = "CSPFK092E No secrets were successfully retrieved"
9699
const CSPFK068E string = "CSPFK068E Retrieved secrets did not include secret '%s' requested by secret group '%s'"
97100

98101
// URL Parsing

pkg/log/messages/info_messages.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,4 @@ const CSPFK032I string = "CSPFK032I Detected removed keys from conjur-map in sec
4242
const CSPFK033I string = "CSPFK033I Removing key '%s' from Kubernetes secret '%s' as it was removed from conjur-map"
4343
const CSPFK034I string = "CSPFK034I Allow secret '%s' without conjur-map, continue..."
4444
const CSPFK035I string = "CSPFK035I Exceeded max debounce delay, providing secrets (eventCount: %d)"
45+
const CSPFK036I string = "CSPFK036I V2 batch retrieval succeeded: retrieved %d secrets"

pkg/secrets/clients/conjur/conjur_client.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,11 @@ func NewConjurClient(tokenData []byte) (ConjurClient, error) {
3232
return nil, log.RecordedError(messages.CSPFK032E, err.Error())
3333
}
3434

35-
return client, nil
35+
// Wrap the client to provide API V2 with V1 fallback
36+
wrapper := &conjurClientWrapper{
37+
client: client,
38+
useV2: true,
39+
}
40+
41+
return wrapper, nil
3642
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package conjur
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"strings"
7+
8+
"github.com/cyberark/conjur-api-go/conjurapi"
9+
"github.com/cyberark/conjur-authn-k8s-client/pkg/log"
10+
11+
"github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages"
12+
)
13+
14+
// conjurClientWrapper wraps the conjur-api-go Client to provide
15+
// V2 batch retrieval with fallback to V1 for backwards compatibility
16+
type conjurClientWrapper struct {
17+
client *conjurapi.Client
18+
useV2 bool
19+
}
20+
21+
// RetrieveBatchSecretsSafe attempts to use V2 batch retrieval API first,
22+
// falling back to V1 if V2 is not available
23+
func (w *conjurClientWrapper) RetrieveBatchSecretsSafe(variableIDs []string) (map[string][]byte, error) {
24+
if w.useV2 {
25+
secrets, err := w.retrieveBatchSecretsV2(variableIDs)
26+
if err != nil {
27+
if isV2NotAvailableError(err) {
28+
log.Warn(messages.CSPFK090E, err.Error())
29+
w.useV2 = false
30+
} else {
31+
return nil, err
32+
}
33+
} else {
34+
return secrets, nil
35+
}
36+
}
37+
38+
log.Debug(messages.CSPFK017D, "Using V1 batch retrieval")
39+
return w.client.RetrieveBatchSecretsSafe(variableIDs)
40+
}
41+
42+
// retrieveBatchSecretsV2 uses the V2 batch retrieval API
43+
func (w *conjurClientWrapper) retrieveBatchSecretsV2(variableIDs []string) (map[string][]byte, error) {
44+
v2Client := w.client.V2()
45+
if v2Client == nil {
46+
return nil, fmt.Errorf("V2 client not available")
47+
}
48+
49+
// V2 API expects variable paths without the account:variable: prefix
50+
// Normalize IDs: "account:variable:path/to/secret" -> "path/to/secret"
51+
normalizedIDs := make([]string, len(variableIDs))
52+
originalIDMap := make(map[string]string) // maps normalized ID -> original ID
53+
54+
for i, id := range variableIDs {
55+
normalizedID := normalizeVariableIdForV2(id)
56+
normalizedIDs[i] = normalizedID
57+
originalIDMap[normalizedID] = id
58+
}
59+
60+
batchResp, err := v2Client.BatchRetrieveSecrets(normalizedIDs)
61+
if err != nil {
62+
return nil, err
63+
}
64+
65+
secrets := make(map[string][]byte)
66+
var failedSecrets []string
67+
68+
for _, secret := range batchResp.Secrets {
69+
if secret.Status == http.StatusOK {
70+
// Success - map back to original ID format
71+
originalID := originalIDMap[secret.ID]
72+
if originalID == "" {
73+
originalID = secret.ID
74+
}
75+
secrets[originalID] = []byte(secret.Value)
76+
} else {
77+
// Failed - track for error reporting (use original ID if available)
78+
originalID := originalIDMap[secret.ID]
79+
if originalID == "" {
80+
originalID = secret.ID
81+
}
82+
failedSecrets = append(failedSecrets, fmt.Sprintf("%s (status: %d)", originalID, secret.Status))
83+
}
84+
}
85+
86+
if len(failedSecrets) > 0 {
87+
log.Warn(messages.CSPFK091E, strings.Join(failedSecrets, ", "))
88+
}
89+
90+
if len(secrets) == 0 {
91+
return nil, fmt.Errorf(messages.CSPFK092E)
92+
}
93+
94+
log.Info(messages.CSPFK036I, len(secrets))
95+
return secrets, nil
96+
}
97+
98+
func (w *conjurClientWrapper) Resources(filter *conjurapi.ResourceFilter) ([]map[string]interface{}, error) {
99+
return w.client.Resources(filter)
100+
}
101+
102+
func (w *conjurClientWrapper) Cleanup() {
103+
w.client.Cleanup()
104+
}
105+
106+
// isV2NotAvailableError checks if the error indicates V2 API is not available
107+
func isV2NotAvailableError(err error) bool {
108+
if err == nil {
109+
return false
110+
}
111+
112+
errMsg := err.Error()
113+
indicators := []string{
114+
"not supported in",
115+
"404",
116+
}
117+
118+
for _, indicator := range indicators {
119+
if strings.Contains(errMsg, indicator) {
120+
return true
121+
}
122+
}
123+
return false
124+
}
125+
126+
// normalizeVariableIdForV2 converts a variable ID to the format expected by V2 API
127+
// The V2 API expects just the variable path (e.g., "secrets/test_secret")
128+
// not the full identifier format (e.g., "my-account:variable:secrets/test_secret")
129+
func normalizeVariableIdForV2(fullVariableId string) string {
130+
variableIdParts := strings.SplitN(fullVariableId, ":", 3)
131+
if len(variableIdParts) == 3 {
132+
// Format is "account:variable:path" -> return just "path"
133+
return variableIdParts[2]
134+
}
135+
// Already in correct format or unknown format, return as-is
136+
return fullVariableId
137+
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package conjur
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestIsV2NotAvailableError(t *testing.T) {
11+
testCases := []struct {
12+
name string
13+
err error
14+
expected bool
15+
}{
16+
{
17+
name: "nil error",
18+
err: nil,
19+
expected: false,
20+
},
21+
{
22+
name: "version too old error",
23+
err: fmt.Errorf("not supported in Conjur versions older than 1.24.0"),
24+
expected: true,
25+
},
26+
{
27+
name: "404 not found",
28+
err: fmt.Errorf("404 endpoint not found"),
29+
expected: true,
30+
},
31+
{
32+
name: "error with 'not supported in' phrase",
33+
err: fmt.Errorf("Batch Retrieve Secrets API is not supported in this version"),
34+
expected: true,
35+
},
36+
}
37+
38+
for _, tc := range testCases {
39+
t.Run(tc.name, func(t *testing.T) {
40+
result := isV2NotAvailableError(tc.err)
41+
assert.Equal(t, tc.expected, result)
42+
})
43+
}
44+
}
45+
46+
func TestNormalizeVariableIdForV2(t *testing.T) {
47+
testCases := []struct {
48+
name string
49+
input string
50+
expected string
51+
}{
52+
{
53+
name: "full identifier with account and variable prefix",
54+
input: "my-account:variable:secrets/password",
55+
expected: "secrets/password",
56+
},
57+
{
58+
name: "full identifier with nested path",
59+
input: "my-account:variable:app/db/credentials/password",
60+
expected: "app/db/credentials/password",
61+
},
62+
{
63+
name: "already normalized path",
64+
input: "secrets/test_secret",
65+
expected: "secrets/test_secret",
66+
},
67+
{
68+
name: "simple variable name",
69+
input: "password",
70+
expected: "password",
71+
},
72+
{
73+
name: "variable with spaces",
74+
input: "my-account:variable:secrets/var with spaces",
75+
expected: "secrets/var with spaces",
76+
},
77+
{
78+
name: "variable with special characters",
79+
input: "my-account:variable:secrets/var+with+pluses",
80+
expected: "secrets/var+with+pluses",
81+
},
82+
{
83+
name: "variable with colon in path",
84+
input: "my-account:variable:secrets/key:value",
85+
expected: "secrets/key:value",
86+
},
87+
{
88+
name: "empty string",
89+
input: "",
90+
expected: "",
91+
},
92+
}
93+
94+
for _, tc := range testCases {
95+
t.Run(tc.name, func(t *testing.T) {
96+
result := normalizeVariableIdForV2(tc.input)
97+
assert.Equal(t, tc.expected, result)
98+
})
99+
}
100+
}

0 commit comments

Comments
 (0)