-
Notifications
You must be signed in to change notification settings - Fork 17
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
This Action does not work with extracted providers #86
Comments
I took a very superficial look at one repo that uses this action (https://github.com/solvahol/dns). Up to commit 536c0bb550e3c527c59340751d5f0cf0ed33a946 you had a imho this would be the proper place to address your issue (in a "package install" or "dependency solution" step) rather than expecting that some package auto-installs some other packages just due to input in the payload (the configuration file). Personally, I would always refrain from any "auto-install-something-based-on-input-data" pattern for the sake of security, safety and readability. ymmv. (edit / just to clarify: I came here from the link at octodns/octodns#622 and was curious where OctoDNS users are affected by the split, and if so -- how and why) |
Thank you @istr 🙇
I appreciate you taking the time to look into this and share your recommendation. I've been thinking of installing OctoDNS and installing provider dependencies as part of this Action, and now I see they needn't be. Removing those to a step in the user's workflow would simplify their control of the environment, and their dependency and vulnerability management. And it'd remove complexity from this project. While eliminating this issue. 👍 |
The proposed solution of pinning octodns to
fails with the following error:
Any suggestions on how this could be fixed, at least on a temporary basis? EDIT: this has been temporarily fixed by adding the following to a
and the following step to the job:
|
@barnumbirr Thank you for writing this up and describing workaround you used 🙇 I'll make time this coming weekend (18-19 Jun) to fix this in the octodns-sync Action and its documentation. Do you think these steps would suffice?:
|
I'd prefer a solution in which package requirements don't have to be redefined multiple times. With the temporary solution I've described above,
That would be fine.
Perfect! |
👋 I pushed changes to the If you'd like to test the uses: solvaholic/octodns-sync@issue86 To see a test already set up, check out the validate and deploy workflows in solvahol/dns and solvahol/dns#35. I plan to get these changes, or improved versions, in to |
👋 Merging #89 closed this issue. It removed octodns installation to a workflow step. The user's workflow can install octodns and required providers any way the user likes. Example workflows in documentation suggest installing from requirements.txt, as @barnumbirr described in #86 (comment). If you have questions about this, or continue to see problems, please re-open this issue or create a new one 🙇 |
@solvaholic thanks a bunch. Limitied testing on my end seems to confirm this works as expected 🎉 . |
Thanks for following up and letting me know @barnumbirr!
Yes! I've messed up releases in this repo, and I think there are some feedback to integrate, so I want to properly get my head into it. I think this'll become 3.0 on 21 Jun. I feel silly going up to 3.x but the changes for this will break things for users who specify |
Description
Providers are being extracted from octodns/octodns to their own repositories in the octodns org: octodns/octodns#622
Currently, this Action does not work with providers that have already been extracted. That means, for example, this Action cannot be used with octodns/octodns at the default branch.
Expected Behavior
This Action should Just Work with octodns: If an extracted provider is used in the config, then install it.
Actual Behavior
If the provider has been installed by other means, this Action continues to function. If not, then an error probably similar to:
Possible Fix
Could detect providers used in config, and install them.
Or install all of the extracted providers.
Or stick with v0.9.14 forever.
Steps to Reproduce
Context
Can't use the latest octodns/octodns with this Action.
This workflow definition failed because I didn't install octodns_route53:
This workflow definition ran successfully:
Your Environment
The text was updated successfully, but these errors were encountered: