control-service: secrets service implementation #2241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Iterative change for VEP-1493. The service is still not publich/marked as not implemented due to lack of tests which are going to be implmented as a next step. Added the ability to optionallly configure a hashicorp vault instance, through a feature flag, uri token settings. The change uses the spring cloud library as a client library for connecting to Hashicorp Vault. I've opted out of using a Vault based Spring CRUD repositroy and other Spring Vault facilities as the 2.x version of the spring libraries cannot work with a vault k/v secrets storage v.2 which is now the default and migrating to spring 3.0 would require a lot of other changes across the whole service. Signed-off-by: Dako Dakov <[email protected]>
Fix codacy checks Signed-off-by: Dako Dakov <[email protected]>
Fix codacy violations. Signed-off-by: Dako Dakov <[email protected]>
mivanov1988
reviewed
Jun 13, 2023
projects/control-service/projects/pipelines_control_service/build.gradle
Outdated
Show resolved
Hide resolved
mivanov1988
reviewed
Jun 13, 2023
...ol_service/src/main/java/com/vmware/taurus/secrets/controller/DataJobsSecretsController.java
Outdated
Show resolved
Hide resolved
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
mivanov1988
approved these changes
Jun 13, 2023
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
murphp15
reviewed
Jun 13, 2023
...es_control_service/src/main/java/com/vmware/taurus/exception/DataJobPropertiesException.java
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...ts/pipelines_control_service/src/main/java/com/vmware/taurus/secrets/service/JobSecrets.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...lines_control_service/src/main/java/com/vmware/taurus/secrets/service/JobSecretsService.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...lines_control_service/src/main/java/com/vmware/taurus/secrets/service/JobSecretsService.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...lines_control_service/src/main/java/com/vmware/taurus/secrets/service/JobSecretsService.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...lines_control_service/src/main/java/com/vmware/taurus/secrets/service/JobSecretsService.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...ol_service/src/main/java/com/vmware/taurus/secrets/controller/DataJobsSecretsController.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...es_control_service/src/main/java/com/vmware/taurus/exception/DataJobPropertiesException.java
Outdated
Show resolved
Hide resolved
murphp15
reviewed
Jun 13, 2023
...lines_control_service/src/main/java/com/vmware/taurus/exception/DataJobSecretsException.java
Outdated
Show resolved
Hide resolved
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
murphp15
reviewed
Jun 14, 2023
projects/control-service/projects/pipelines_control_service/build.gradle
Outdated
Show resolved
Hide resolved
murphp15
approved these changes
Jun 14, 2023
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
Address review feedback. Signed-off-by: Dako Dakov <[email protected]>
dakodakov
deleted the
person/ddakov/secrets-service-initial-implementation
branch
| package com.vmware.taurus.exception; | ||
|
|
||
| /** Exception thrown, when a secret storage has not been configured */ | ||
| public class SecretStorageNotConfiguredException extends ExternalSystemError { |
There was a problem hiding this comment.
External System error is 503. That does not seem suitable here.
503 is generally a re-triable error. And often would be re-tried. But as the secrets feature is disbaled re-trying is unnecessary.
403 Forbidden (means The server understood the request, but is refusing to fulfill it.)
501 Not Implemented
seem more suitable for this case. I'd vote for 501
There was a problem hiding this comment.
Makes sense. I'll fix it in the upcoming PR for the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Iterative change for VEP-1493.
The service is still not publich/marked as not implemented due to lack of tests which are going to be implmented as a next step.
Added the ability to optionallly configure a hashicorp vault instance, through a feature flag, uri token settings.
The change uses the spring cloud library as a client library for connecting to Hashicorp Vault.
I've opted out of using a Vault based Spring CRUD repositroy and other Spring Vault facilities as the 2.x version of the spring libraries cannot work with a vault k/v secrets storage v.2 which is now the default and migrating to spring 3.0 would require a lot of other changes across the whole service.