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

NOISSUE - Vault operations with app role authentication #2084

Merged
merged 40 commits into from
Feb 20, 2024

Conversation

arvindh123
Copy link
Contributor

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

  • Related Issue #
  • Resolves #

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: arvindh123 <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: arvindh123 <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: arvindh123 <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: arvindh123 <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 66e1d3f.

Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 34ac047.

Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 7db464f.

Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 290252d.

Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 2523e61.

Signed-off-by: Arvindh <arvindh91@gmail.com>
This reverts commit 456b627.

Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
@arvindh123 arvindh123 marked this pull request as ready for review February 19, 2024 17:17
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Comment on lines 19 to 20


Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove multiple empty lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,21 @@
#!/usr/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider renaming all copy files (maybe to vault_copy_certs and vault_copy_env - also, use underscore for the sake of consistency) because the copy suffix can be confused with accidentally copied files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

file name replace - with underscore

vault write -namespace=${MG_VAULT_NAMESPACE} -address=${MG_VAULT_ADDR} ${MG_VAULT_PKI_PATH}/config/crl expiry="5m" ocsp_disable=false ocsp_expiry=0 auto_rebuild=true auto_rebuild_grace_period="2m" enable_delta=true delta_rebuild_interval="1m"
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove multiple empty lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +51 to +53
if ($dynamic_server_name = '') {
set $dynamic_server_name "localhost";
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we set it by default to localhost, there is no need for this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed approach

Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
Signed-off-by: Arvindh <arvindh91@gmail.com>
@dborovcanin dborovcanin merged commit ab4206c into absmach:main Feb 20, 2024
5 checks passed
@arvindh123 arvindh123 mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants