Skip to content

VAULT-35602: Adding Enos OpenLDAP test#30801

Merged
tvo0813 merged 3 commits into
mainfrom
enos/VAULT-35602
Jul 23, 2025
Merged

VAULT-35602: Adding Enos OpenLDAP test#30801
tvo0813 merged 3 commits into
mainfrom
enos/VAULT-35602

Conversation

@tvo0813
Copy link
Copy Markdown
Collaborator

@tvo0813 tvo0813 commented May 31, 2025

Description

VAULT-35602: Adding Enos OpenLDAP test

TODO only if you're a HashiCorp employee

  • Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
    • LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@tvo0813 tvo0813 requested a review from a team as a code owner May 31, 2025 00:09
@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch from 79ce381 to 2d4e10f Compare May 31, 2025 00:09
@github-actions github-actions Bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label May 31, 2025
@tvo0813 tvo0813 added pr/no-changelog pr/no-milestone backport/ent/1.16.x+ent and removed hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed labels May 31, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 31, 2025 •
edited
Loading

CI Results:
All Go tests succeeded! ✅

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 31, 2025 •
edited
Loading

Build Results:
All builds succeeded! ✅

@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch from 2d4e10f to 9ac1184 Compare June 2, 2025 22:00
@github-actions github-actions Bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 2, 2025
@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch 5 times, most recently from b9108e4 to 10d75cb Compare June 4, 2025 00:31
brewgator
brewgator suggested changes Jun 6, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@brewgator brewgator left a comment

Choose a reason for hiding this comment

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

Here we write to a non-existent fake LDAP server. It might be worth updating this to point to the one Enos has spun up.

@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch 9 times, most recently from 114972c to 01fd627 Compare June 12, 2025 17:04
@tvo0813 tvo0813 reopened this Jun 18, 2025
tvo0813
tvo0813 commented Jun 18, 2025
View reviewed changes
Comment thread enos/modules/verify_secrets_engines/scripts/ldap-configs.sh
@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch 8 times, most recently from 2cf40b3 to b3a7b83 Compare June 20, 2025 01:17
@tvo0813 tvo0813 requested a review from brewgator June 20, 2025 16:40
@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch from b3a7b83 to c8c003f Compare June 20, 2025 16:40
ryancragun
ryancragun reviewed Jun 20, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

I'm excited to get this in ASAP! A few mostly naming related bits of feedback.

Comment thread enos/enos-descriptions.hcl Outdated
Comment thread enos/enos-scenario-agent.hcl Outdated
Comment thread enos/modules/backend_test_servers/main.tf Outdated
Comment thread enos/modules/backend_test_servers/scripts/setup_docker.sh Outdated
Comment thread enos/enos-scenario-autopilot.hcl Outdated
Comment thread enos/modules/backend_test_servers/scripts/setup_openldap.sh Outdated
Comment thread enos/modules/target_ec2_instances/main.tf Outdated
Comment thread enos/modules/verify_secrets_engines/modules/create/aws/aws.tf Outdated
Comment thread enos/modules/verify_secrets_engines/scripts/ldap-configs.sh Outdated
Comment thread enos/modules/verify_secrets_engines/scripts/ldap-configs.sh Outdated
@tvo0813 tvo0813 force-pushed the enos/VAULT-35602 branch 2 times, most recently from 61a3701 to 1746120 Compare June 25, 2025 21:24
ryancragun
ryancragun requested changes Jun 26, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

This is looking pretty close. I definitely don't want to expose SSH on the integration machines to all addresses, so that will have to be resolved before we merge. I put most of my scenario based feedback on the agent scenario but it applies to all scenarios.

Comment thread enos/enos-globals.hcl Outdated
Comment thread enos/enos-scenario-agent.hcl Outdated
Comment thread enos/enos-scenario-agent.hcl Outdated
Comment thread enos/enos-modules.hcl Outdated
Comment thread enos/modules/backend_test_servers/main.tf Outdated
Comment thread enos/enos-scenario-agent.hcl Outdated
Comment thread enos/modules/backend_test_servers/scripts/set-up-docker.sh Outdated
Comment thread enos/modules/target_ec2_instances/main.tf Outdated
Comment thread enos/modules/verify_secrets_engines/modules/create/main.tf Outdated
ryancragun
ryancragun reviewed Jul 2, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ryancragun ryancragun left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few more tweaks and a couple of DRY improvements and I think this is ready. When the rest of the feedback is addressed it'd be good to do a test run of this branch in enterprise to ensure we haven't regressed somewhere.

Comment thread enos/enos-globals.hcl Outdated
Comment thread enos/modules/target_set_up_external_integration/main.tf Outdated
Comment thread enos/modules/target_set_up_external_integration/scripts/set-up-docker.sh Outdated
Comment thread enos/modules/target_set_up_external_integration/scripts/set-up-openldap.sh Outdated
Comment thread enos/modules/verify_secrets_engines/modules/read/ldap.tf Outdated
Comment thread enos/modules/verify_secrets_engines/modules/create/main.tf Outdated
Comment thread enos/modules/verify_secrets_engines/scripts/ldap-configs.sh Outdated
brewgator
brewgator previously approved these changes Jul 3, 2025
View reviewed changes
@tvo0813
Copy link
Copy Markdown
Collaborator Author

tvo0813 commented Jul 4, 2025

tested change on enterprise and all tests passed:
https://github.com/hashicorp/vault-enterprise/pull/8296

brewgator
brewgator previously approved these changes Jul 7, 2025
View reviewed changes
brewgator
brewgator previously approved these changes Jul 9, 2025
View reviewed changes
brewgator
brewgator previously approved these changes Jul 11, 2025
View reviewed changes
charlesn-hc
charlesn-hc previously approved these changes Jul 14, 2025
View reviewed changes
brewgator
brewgator previously approved these changes Jul 17, 2025
View reviewed changes
ryancragun
ryancragun reviewed Jul 23, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ryancragun ryancragun 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! Let's just do that rename and merge this.

Comment thread enos/enos-modules.hcl Outdated
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryancragun ryancragun ryancragun approved these changes

@charlesn-hc charlesn-hc charlesn-hc left review comments

+1 more reviewer

@brewgator brewgator brewgator left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog pr/no-milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tvo0813 @brewgator @ryancragun @charlesn-hc @stevendpclark @ej-hashi