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

[FEAT] add OAuth key integration and additional URL-style params to --auth-key flags (ephemeral & preauthorized) #399

Merged
merged 19 commits into from
Dec 21, 2023

Conversation

@McSim85
Copy link
Contributor Author

McSim85 commented Dec 19, 2023

Can you please duplicate the default directory/test suite under molecule/, rename the new directory to oauth, and modify

tailscale_authkey: "{{ lookup('ansible.builtin.env', 'TAILSCALE_CI_KEY') }}"

inside the new test directory to look for the env var TAILSCALE_OAUTH_CLIENT_SECRET instead of TAILSCALE_CI_KEY?
Also be sure to rename the scenario to oauth as well:

The way tests work in this repo (to securely use my secrets in PRs), the new test won't get triggered in this PR but I'll manually verify its correctness before merging.

this is done in 23f6a82 I am not super familiar with molecule. I hope I did renaming correctly. 🙏

Also, in 1467653 added /oauth/ tests to linter's exclude_paths

Hi @artis3n
Just a follow-up, in case you missed my commits :)

@artis3n
Copy link
Owner

artis3n commented Dec 20, 2023

This does not work just yet! And to answer the previous comment, Molecule is basically a system for integration tests of Ansible roles.

Running the Molecule test locally with poetry run molecule test --scenario-name oauth (and the TAILSCALE_OAUTH_CLIENT_SECRET env var) results in this error:

image

I've tinkered with the PR and will push up a fix with some minor molecule enhancements as well.

@artis3n
Copy link
Owner

artis3n commented Dec 20, 2023

@McSim85
Copy link
Contributor Author

McSim85 commented Dec 21, 2023

This does not work just yet! And to answer the previous comment, Molecule is basically a system for integration tests of Ansible roles.

Running the Molecule test locally with poetry run molecule test --scenario-name oauth (and the TAILSCALE_OAUTH_CLIENT_SECRET env var) results in this error:

image

I've tinkered with the PR and will push up a fix with some minor molecule enhancements as well.

ohhh, right.
We use --advertise-tags everywhere. So, I did not meet those errors.
--advertise-tags is required with OAuth, unless you use all scopes when granting access.

@McSim85
Copy link
Contributor Author

McSim85 commented Dec 21, 2023

Can you enable the "Allow edits from maintainers" setting so I can push a fix commit?

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

This was enabled already. Can you try to push changes?
image

@artis3n
Copy link
Owner

artis3n commented Dec 21, 2023

Hmm I do see that on my side too, but I'm getting permissions errors. I'm going to merge and put up a subsequent PR with the fixes.

@artis3n artis3n merged commit 172ed73 into artis3n:main Dec 21, 2023
25 checks passed
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.

None yet

2 participants