Add option from datakey to be provided from existing secret

[dan-transmit]

Desired Outcome

The secret doesn't show in the helm values

Implemented Changes

Allow to use existing secrets in the cluster

Connected Issue/Story

Resolves #[relevant GitHub issue(s), e.g. 76]

CyberArk internal issue ID: [insert issue ID]

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be
merged.

Changelog

• The CHANGELOG has been updated, or
• This PR does not include user-facing changes and doesn't require a
  CHANGELOG update

Test coverage

• This PR includes new unit and integration tests to go with the code
  changes, or
• The changes in this PR do not require tests

Documentation

• Docs (e.g. READMEs) were updated in this PR
• A follow-up issue to update official docs has been filed here: [insert issue ID]
• This PR does not require updating any documentation

Behavior

• This PR changes product behavior and has been reviewed by a PO, or
• These changes are part of a larger initiative that will be reviewed later, or
• No behavior was changed with this PR

Security

• Security architect has reviewed the changes in this PR,
• These changes are part of a larger initiative with a separate security review, or
• There are no security aspects to these changes

[szh]

Hi @dan-transmit,

Thank you so much for your PR! I took an initial review pass and it looks great overall. I left a couple of comments for you to address, and we'll try to get this fully reviewed in the next few days.

Thank you!

[szh]

The linter is failing here. I think this needs to default to an empty string instead of null.

[szh]

It would be better to require exactly one of [dataKey, dataKeySecretRef]. This can be accomplished with syntax like this:

"oneOf": [
{
  "required": [
    "dataKey"
  ]
},
{
  "required": [
    "dataKeySecretRef"
   ]
},

See the JSON Schema reference for more details.

[szh]

I think this should default to an empty string. That way if it's not explicitly provided it will fail validation. With this default, I think it would succeed initially and then fail when it couldn't find this secret, which would lead to a confusing error message.

[dan-transmit]

Currently, it fails because two of the values are provided, so I'll just remove it altogether

[szh]

This is looking amazing. Do you time to add a test case to test.sh?

[dan-transmit]

Writing a test would mean to create a whole new setup (it's not like the current script has a reusable interface).
If you think that dataKeySecretRef should be the preferred usage, then I can replace dataKey from the tests with it.

[szh]

That's true. Ideally we'd refactor the tests to make it easier to have multiple test cases. If this isn't something you're interested in doing I totally understand, we just can't merge it without tests. I'm fine to put this on hold for a bit until we can do that internally if that's ok with you.