Skip to content

Conversation

@PatrickEPG
Copy link
Contributor

I've added a module to upload secrets to dvls.

When the a secret already exists, it will be updated. When the secret does not yet exist, a new entry will be created.
I also added some documentation.

This is my first Open-Source contribution so any feedback is very welcome.

@richerlariviere
Copy link

Hello Patrick,

I wanted to sincerely thank you for your contribution, especially since it's your first one. Our society needs more generous people like you, and I wanted to highlight your effort.

By the way, I wanted to clarify with you the mission of the Ansible module for DVLS. To facilitate the maintenance of our various integrations with DVLS, we have decided that these integrations should be used to consume DVLS in read-only mode (we're avoiding write and configuration operations). With read-write management comes inherent complexity, and since DVLS supports multiple entry types, it is unrealistic for us to support all these use cases across our different integrations. Considering that I cannot merge this PR unfortunately.

I'm still interested to know about your specific use-case though. What are the advantages of creating/updating DVLS secrets outside of DVLS UI? I'd really appreciate your feedback regarding that.

Have a nice day,
Richer

@PatrickEPG
Copy link
Contributor Author

Hi Richer,
thank you for the information.

We want to fully automate the deployment of some of our software. So when we deploy the database, users/passwords will be created, which need to be stored somewhere.
Since these Credentials will be consumed by following deployments (software that uses the database) as well as Admins/Support Agents, we figured DVLS is the place to store them.

If I understand correctly, the PowerShell Module https://github.com/Devolutions/SecretManagement.DevolutionsServer allows the creation of secrets and the API allows it as well, which is what I used to implement. Is there a difference in using the PowerShell Module/API directly instead of utilizing Ansible?

Kind regards
Patrick

@richerlariviere
Copy link

Hey @PatrickEPG I think you'll be happy. I just had a talk with our CEO David and we decided to consider your use-case. We are now open to support simple entry types. We will review your PR shortly.

Stay tuned!

Copy link
Contributor

@DannyBedard DannyBedard left a comment

Choose a reason for hiding this comment

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

Works like a charm good job! I requested some changes for the format and the documentation.

PatrickEPG and others added 3 commits February 7, 2025 08:27
except Exception as e:
raise Exception(f"An error occurred while getting vault entries: {e}")

def find_entry_by_name(entries, name):
Copy link
Contributor

@otxi otxi Feb 25, 2025

Choose a reason for hiding this comment

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

FYI this function is useless as it relies on non-functional code. The get_vault_entries(server_base_url, token, vault_id) function is buggy because it doesn't handle pagination so we are limited until only 25 secrets (25 secrets / page) ... The result of a search in the variable entries is incomplete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the information, I also noticed that when I moved the function from fetch_secrets.py. I already have implementing a filter for folders/paths on my to-do list (after this pull request is merged), maybe I will implement a pagination logic then as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for highlighting this issue. The pagination bug is duly noted and will be fixed in a future update.

@DannyBedard DannyBedard merged commit 9f21137 into Devolutions:master Feb 25, 2025
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.

4 participants