Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable SP to retrieve and provide German umlaut chars #500

Merged
merged 1 commit into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

## [1.5.0] - 2023-02-13

### Added
- Adds support for binary secret values and values with special characters.
[cyberark/secrets-provider-for-k8s#500](https://github.com/cyberark/secrets-provider-for-k8s/pull/500)

## [1.4.6] - 2023-01-26

### Security
Expand Down
1 change: 1 addition & 0 deletions deploy/config/k8s/k8s-secret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ stringData:
secret: secrets/test_secret
var_with_spaces: secrets/var with spaces
var_with_pluses: secrets/var+with+pluses
var_with_umlaut: secrets/umlaut
non-conjur-key: some-value
5 changes: 5 additions & 0 deletions deploy/config/k8s/test-env-k8s-rotation.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ spec:
secretKeyRef:
name: test-k8s-secret
key: var_with_pluses
- name: VARIABLE_WITH_UMLAUT_SECRET
valueFrom:
secretKeyRef:
name: test-k8s-secret
key: var_with_umlaut
- name: NON_CONJUR_SECRET
valueFrom:
secretKeyRef:
Expand Down
5 changes: 5 additions & 0 deletions deploy/config/k8s/test-env.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ spec:
secretKeyRef:
name: test-k8s-secret
key: var_with_pluses
- name: VARIABLE_WITH_UMLAUT_SECRET
valueFrom:
secretKeyRef:
name: test-k8s-secret
key: var_with_umlaut
- name: NON_CONJUR_SECRET
valueFrom:
secretKeyRef:
Expand Down
1 change: 1 addition & 0 deletions deploy/config/openshift/k8s-secret.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ stringData:
secret: secrets/test_secret
var_with_spaces: secrets/var with spaces
var_with_pluses: secrets/var+with+pluses
var_with_umlaut: secrets/umlaut
non-conjur-key: some-value
5 changes: 5 additions & 0 deletions deploy/config/openshift/test-env.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ spec:
secretKeyRef:
name: test-k8s-secret
key: var_with_pluses
- name: VARIABLE_WITH_UMLAUT_SECRET
valueFrom:
secretKeyRef:
name: test-k8s-secret
key: var_with_umlaut
- name: NON_CONJUR_SECRET
valueFrom:
secretKeyRef:
Expand Down
5 changes: 5 additions & 0 deletions deploy/dev/config/k8s/secrets-provider-init-container.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ spec:
secretKeyRef:
name: test-k8s-secret
key: var_with_pluses
- name: VARIABLE_WITH_UMLAUT_SECRET
valueFrom:
secretKeyRef:
name: test-k8s-secret
key: var_with_umlaut
- name: NON_CONJUR_SECRET
valueFrom:
secretKeyRef:
Expand Down
5 changes: 5 additions & 0 deletions deploy/dev/config/k8s/secrets-provider-k8s-rotation.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ spec:
secretKeyRef:
name: test-k8s-secret
key: var_with_pluses
- name: VARIABLE_WITH_UMLAUT_SECRET
valueFrom:
secretKeyRef:
name: test-k8s-secret
key: var_with_umlaut
- name: NON_CONJUR_SECRET
valueFrom:
secretKeyRef:
Expand Down
1 change: 1 addition & 0 deletions deploy/policy/load_policies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ done
conjur variable values add secrets/test_secret "some-secret"
conjur variable values add "secrets/var with spaces" "some-secret"
conjur variable values add "secrets/var+with+pluses" "some-secret"
conjur variable values add "secrets/umlaut" "some-secret"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you not need an umlaut in this secret value? Or is this just a placeholder that gets replaced later on?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a placeholder that's replaced when a test case runs [test case][CLI variable load].

conjur variable values add secrets/url "postgresql://test-app-backend.app-test.svc.cluster.local:5432"
conjur variable values add secrets/username "some-user"
conjur variable values add secrets/password "7H1SiSmYp@5Sw0rd"
Expand Down
1 change: 1 addition & 0 deletions deploy/policy/templates/conjur-secrets.template.sh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ cat << EOL
- !variable another_test_secret
- !variable var with spaces
- !variable var+with+pluses
- !variable umlaut
- !variable url
- !variable username
- !variable password
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/bin/bash
set -euxo pipefail

create_secret_access_role

create_secret_access_role_binding

test_secret_is_provided "ÄäÖöÜü" "secrets/umlaut" "VARIABLE_WITH_UMLAUT_SECRET"
8 changes: 4 additions & 4 deletions pkg/secrets/clients/conjur/conjur_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
)

/*
Client for communication with Conjur. In this project it is used only for
batch secrets retrieval so we expose only this method of the client.
Client for communication with Conjur. In this project it is used only for
batch secrets retrieval so we expose only this method of the client.

The name ConjurClient also improves readability as Client can be ambiguous.
The name ConjurClient also improves readability as Client can be ambiguous.
*/
type ConjurClient interface {
RetrieveBatchSecrets([]string) (map[string][]byte, error)
RetrieveBatchSecretsSafe([]string) (map[string][]byte, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure it's totally safe now or are there other corner cases that might sneak in? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: did not come up with the naming here. Having trouble thinking of a better variable name that distinguishes the two without getting into unseen details.

Also, RetrieveBatchSecretsSafe has the same interface as RetrieveBatchSecrets, but doesn't fail on special characters/binary values - maybe RetrieveBatchSecretsSafe should be renamed to RetrieveBatchSecrets, and RetrieveBatchSecrets shouldn't be used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, you're just using a different conjur-api-go method? Nah, that's fine then.

}

func NewConjurClient(tokenData []byte) (ConjurClient, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/secrets/clients/conjur/conjur_secrets_retriever.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func retrieveConjurSecrets(accessToken []byte, variableIDs []string) (map[string
return nil, log.RecordedError(messages.CSPFK033E)
}

retrievedSecretsByFullIDs, err := conjurClient.RetrieveBatchSecrets(variableIDs)
retrievedSecretsByFullIDs, err := conjurClient.RetrieveBatchSecretsSafe(variableIDs)
if err != nil {
return nil, err
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ var testConjurSecrets = map[string]string{
"conjur/var/path2": "secret-value2",
"conjur/var/path3": "secret-value3",
"conjur/var/path4": "secret-value4",
"conjur/var/umlaut": "ÄäÖöÜü",
"conjur/var/binary": "\xf0\xff\x4a\xc3",
"conjur/var/empty-secret": "",
}

Expand Down Expand Up @@ -287,6 +289,42 @@ func TestProvide(t *testing.T) {
),
},
},
{
desc: "Happy path, secret with umlaut characters",
k8sSecrets: k8sStorageMocks.K8sSecrets{
"k8s-secret1": {
"conjur-map": {"secret1": "conjur/var/umlaut"},
},
},
requiredSecrets: []string{"k8s-secret1"},
asserts: []assertFunc{
assertSecretsUpdated(
expectedK8sSecrets{
"k8s-secret1": {"secret1": "ÄäÖöÜü"},
},
expectedMissingValues{},
false,
),
},
},
{
desc: "Happy path, binary secret",
k8sSecrets: k8sStorageMocks.K8sSecrets{
"k8s-secret1": {
"conjur-map": {"secret1": "conjur/var/binary"},
},
},
requiredSecrets: []string{"k8s-secret1"},
asserts: []assertFunc{
assertSecretsUpdated(
expectedK8sSecrets{
"k8s-secret1": {"secret1": "\xf0\xff\x4a\xc3"},
},
expectedMissingValues{},
false,
),
},
},
{
desc: "K8s Secrets maps to a non-existent Conjur secret",
k8sSecrets: k8sStorageMocks.K8sSecrets{
Expand Down