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

Add P2F docs for using custom templates #391

Merged
merged 1 commit into from
Nov 17, 2021
Merged

Conversation

john-odonnell
Copy link
Contributor

@john-odonnell john-odonnell commented Nov 16, 2021

Depends on #390

Desired Outcome

From ONYX-14031:

  • Overview of how custom templates work and how to reference secrets
  • Example custom template
  • Description of notable template functions for this use of Golang, with link to external Golang template docs for full details.
  • Include note that the initial validation uses fake data to provide early and secure feedback with complete messages, so templates that rely on secret values (i.e. sprintf to format a number) may fail at this stage. But they still would work fine with real secret data.

Implemented Changes

Makes a few additions to PUSH_TO_FILE.md:

  • an entry in the Reference Table of Configuration Annotations for conjur.org/secret-file-template.{secret-group}
  • section Custom Templates For Secret Files, which includes
    • overview of custom templates
    • detail referencing secrets in custom templates
    • note of template functions in Go's text/template package
    • description of "double-pass" render procedure and a note suggesting against using templates that rely on secret values
    • example custom templates and outputs

Connected Issue/Story

CyberArk internal issue link: ONYX-14031

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

@john-odonnell john-odonnell force-pushed the custom-template-docs branch 2 times, most recently from ee50e7c to f3ac451 Compare November 16, 2021 18:45
@john-odonnell john-odonnell changed the base branch from main to ONYX-13494 November 16, 2021 18:47
@diverdane diverdane force-pushed the ONYX-13494 branch 4 times, most recently from a6cbfb6 to d38e934 Compare November 17, 2021 14:53
Copy link
Contributor

@szh szh left a comment

Choose a reason for hiding this comment

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

Looks great! Just need to wait for #390 now

@diverdane diverdane force-pushed the ONYX-13494 branch 3 times, most recently from a29a4c7 to c68b74e Compare November 17, 2021 18:57
Copy link
Contributor

@diverdane diverdane left a comment

Choose a reason for hiding this comment

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

Nice work on documenting the custom template function!
Just a couple of minor comments, otherwise looks good-to-go.

```
{"api-url":"value-dev/redis/api-url","admin-username":"value-dev/redis/username","admin-password
":"value-dev/redis/password"}
username | admin
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a couple of words before this example secret file output, e.g. "The Secrets Provider will create a secret file with the content:" or something like that.

Same comment applies to other sample custom template output below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this in. For each custom template example, I made sure to include the conjur.org/secret-file-path annotation, because it's required as of now, and I didn't want to spark confusion. Added comments before the output snippets:

Secrets Provider will render the following content for the file /conjur/secrets/<added-file>

conjur.org/secret-file-path.cache: "./testdata/redis.sh"
conjur.org/secret-file-format.cache: "bash"
conjur.org/secret-file-template.iterative-reference: |
{{- range $index, $secret := .SecretsArray -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

It might help users to have some background context for the .SecretsArray reference here. Maybe something like:

"Here, the .SecretsArray is a reference to Secrets Provider's internal array of secrets that have been retrieved from Conjur. For each entry in this array, there is a secret Alias and a secret Value field that can be referenced in the custom template."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, good idea. Added!

@john-odonnell john-odonnell force-pushed the custom-template-docs branch 2 times, most recently from a12a19c to fb0c8b6 Compare November 17, 2021 21:11
Base automatically changed from ONYX-13494 to main November 17, 2021 22:30
Copy link
Contributor

@diverdane diverdane left a comment

Choose a reason for hiding this comment

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

Approving, but there is one minor typo, otherwise GTG!

PUSH_TO_FILE.md Outdated

Here, `.SecretsArray` is a reference to Secret Provider's internal array of
secrets that have been retrieved from Conjur. For each entry in this array,
there is a secret `Alias` and `Value` field tat can be referenced in the custom
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have to rebase anyway, there's a typo: tat can be referenced should be that can be referenced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rats, my bad. Fixed, rebased, and (officially) opened for review.

@john-odonnell john-odonnell marked this pull request as ready for review November 17, 2021 22:46
@john-odonnell john-odonnell requested review from a team as code owners November 17, 2021 22:46
@codeclimate
Copy link

codeclimate bot commented Nov 17, 2021

Code Climate has analyzed commit 6922ede and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 91.4% (0.0% change).

View more on Code Climate.

@john-odonnell john-odonnell merged commit 972a813 into main Nov 17, 2021
@john-odonnell john-odonnell deleted the custom-template-docs branch November 18, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants