-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: Add salt-api ldap eauth tests. #269
Conversation
da5965b
to
0077415
Compare
Wow! That was fast! Thank you very much for adding this test! I think it is perfect, and a great documentation complement for people needing a full example of how to setup the image with an ldap db. You can call this test directly from the salt-api section of the workflow: - name: Execute salt-api tests
if: always()
run: |
tests/salt-api/test.sh
tests/salt-api/salt-api-ldap.sh Currently all tests are failing because of Salt Project migration: https://saltproject.io/blog/upcoming-salt-project-docs-and-repo-migration/ I hope the migration finishes soon so we can run these tests without errors. |
0077415
to
6ee4635
Compare
It was actually much easier than I expected to get working due to the container already having default baseDN, user accounts etc set up. I've updated the tests to run |
18c4c34
to
2eea147
Compare
Hi @mrichar1, could you rebase your branch with the latest changes in the |
838382b
to
0bf4835
Compare
That's the rebase completed and the shellcheck fix added and pushed. |
0bf4835
to
c4741ed
Compare
Thank you, @mrichar1! Your test has passed, so I'm going to merge the PR! I have to solve an issue with the gitfs tests, but I think it must be something related with secrets permissions. Thank you for your contribution! |
A first pass at adding salt-api ldap integration tests as promised.
The
osixia/openldap
container comes with 2 'builtin' users:admin
andreadonly
, and we add an ldap group calledsaltadmins
containing thereadonly
user.We then set
salt-api.conf
to connect to the LDAP server, and set theeauth
rules to give access 'directly' to theadmin
user and also to members of thesaltadmins
group.We then get a token and run a command via the for both users, testing both eauth rules.
I have tweaked things to try to handle the start/stop of the openldap container and cleanup neatly - but you might prefer the existing
cleanup
function being extended to also shut down the openldap container, if found?I also haven't added this test to the github workflow as I wasn't sure if it should go in as it's own entry, or will be called from the existing
salt-api/test.sh
Let me know what you think, and if you have any other changes you would like to see.