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.

Sign up for GitHub

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

Jump to bottom

Retrieve Secrets for secrets group #363

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Retrieve Secrets for secrets group #363

merged 2 commits into from
Oct 14, 2021

Conversation

rpothier
Copy link
Contributor

What does this PR do?

Parse the secrets group structure and retrieve the secrets from Conjur

What ticket does this PR close?

Resolves #343

Checklists

Change log

  • 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, and/or there is a follow-on issue to update docs, or
  • This PR does not require updating any documentation

@rpothier rpothier requested review from a team as code owners September 28, 2021 13:26
@rpothier rpothier self-assigned this Sep 28, 2021
@rpothier rpothier marked this pull request as draft September 28, 2021 13:26
@rpothier rpothier force-pushed the 343_retrieve_secrets branch 2 times, most recently from b0b1b83 to c564b6b Compare September 30, 2021 15:48
@rpothier rpothier marked this pull request as ready for review September 30, 2021 15:51
@rpothier rpothier force-pushed the 343_retrieve_secrets branch from c564b6b to e07a88d Compare September 30, 2021 16:54
@rpothier rpothier changed the title retrieve secrets Retrieve Secrets for secrets group Oct 1, 2021
@rpothier rpothier force-pushed the 343_retrieve_secrets branch from e07a88d to f49b7fe Compare October 2, 2021 01:16
john-odonnell
john-odonnell reviewed Oct 4, 2021
View reviewed changes
Copy link
Contributor

@john-odonnell john-odonnell left a comment

Choose a reason for hiding this comment

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

Just a couple small comments. Looks good otherwise!

pkg/secrets/pushtofile/secret_groups.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
diverdane
diverdane reviewed Oct 4, 2021
View reviewed changes
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.

Looks good so far! I'm still digging through the UT but wanted to post a partial review.

pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/secret_groups.go Outdated Show resolved Hide resolved
@rpothier rpothier force-pushed the 343_retrieve_secrets branch 3 times, most recently from 47aed22 to fba5181 Compare October 4, 2021 19:26
diverdane
diverdane reviewed Oct 4, 2021
View reviewed changes
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets_test.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets_test.go Outdated Show resolved Hide resolved
@rpothier rpothier force-pushed the 343_retrieve_secrets branch 3 times, most recently from ad6ba0a to 6e2c10d Compare October 5, 2021 19:29
doodlesbykumbi
doodlesbykumbi suggested changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@doodlesbykumbi doodlesbykumbi left a comment

Choose a reason for hiding this comment

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

I've left a number of comments and suggestions. Happy to chat more about them online.

pkg/secrets/pushtofile/retrieve_secrets_test.go Outdated Show resolved Hide resolved
pkg/secrets/clients/conjur/mocks/conjur_secrets_retriever.go Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets_test.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets_test.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
@rpothier rpothier force-pushed the 343_retrieve_secrets branch 5 times, most recently from ac9c8ed to 2e2f897 Compare October 13, 2021 13:26
@rpothier rpothier force-pushed the 343_retrieve_secrets branch 2 times, most recently from fad588d to 615fdda Compare October 13, 2021 14:46
@doodlesbykumbi @rpothier
Refactor mocks
307f447 
Moves Conjur mocks to dedicated directory. Binds mocks to struct instances instead of using global variables
@rpothier rpothier force-pushed the 343_retrieve_secrets branch from 615fdda to e0f447e Compare October 13, 2021 18:00
diverdane
diverdane reviewed Oct 13, 2021
View reviewed changes
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.

Looks good, only a couple of VERY minor gofmt/golint issues.

pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
pkg/secrets/pushtofile/retrieve_secrets.go Outdated Show resolved Hide resolved
@rpothier rpothier force-pushed the 343_retrieve_secrets branch from e0f447e to 1bff883 Compare October 14, 2021 12:39
diverdane
diverdane approved these changes Oct 14, 2021
View reviewed changes
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.

LGTM!!!

@rpothier rpothier force-pushed the 343_retrieve_secrets branch from 1bff883 to 9edb714 Compare October 14, 2021 15:28
@codeclimate
Copy link

codeclimate bot commented Oct 14, 2021

Code Climate has analyzed commit 9edb714 and detected 0 issues on this pull request.

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

This pull request will bring the total coverage in the repository to 92.6% (-1.6% change).

View more on Code Climate.

@doodlesbykumbi doodlesbykumbi merged commit 2106df9 into main Oct 14, 2021
@doodlesbykumbi doodlesbykumbi deleted the 343_retrieve_secrets branch October 14, 2021 16:58
@jtuttle jtuttle mentioned this pull request Nov 12, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-odonnell john-odonnell john-odonnell left review comments

@doodlesbykumbi doodlesbykumbi doodlesbykumbi approved these changes

@diverdane diverdane diverdane approved these changes

Assignees

@rpothier rpothier

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

There are defined data structures for rendering and writing secrets files in M1
4 participants
@rpothier @doodlesbykumbi @diverdane @john-odonnell