-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Rundeck Provider #2412
Rundeck Provider #2412
Conversation
@phinze This is now feature-complete, and I'd be really grateful for a code review. |
Well, the documentation's awesome, and it's got tests. Can't be that bad! |
@phinze @catsby @mitchellh I know this is a bit of a big patch and probably hard for you guys to review since you probably don't use rundeck... I'd be interested to know at least if you guys consider this to be a worthy thing to be built in to Terraform or if you'd rather I just pursued making a separate plugin for this. I personally think Rundeck is a good companion to the rest of "the Hashicorp stack" but I'm happy to spin up my own plugin repo for this if you'd rather keep Terraform focused on cloud infrastructure providers. |
As a heavy rundeck user, I am very glad to see this :) The docs are great as well! |
Type: schema.TypeString, | ||
Required: true, | ||
Description: "URL of the root of the target Rundeck server.", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to provide a RUNDECK_URL
env var here? I'm personally all about fully env-var-configurable providers - and I almost never write a provider {}
block into my configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess not... my own personal style is to put the stuff that talks about the user in the env vars and the stuff that talks about the infrastructure in the config, but no reason to force my style on everyone.
(My motivation: usually in my world the targets of these things come from other resources like consul_keys
and terraform_remote_state
, rather than being set directly by the user... though that of course leads to the issue I noted in #2976.)
Finally got around to this for you, @apparentlymart - amazing work, as I've come to realize is standard operating procedure for you! 😀 A few inline nits, but generally this looks good to go! Let me know what you think of the comments and we'll get this sucker merged. |
Thanks for the thorough review, @phinze! I'll try to take action on your feedback soon... unfortunately most likely to be next week since I'm focused elsewhere right now. In the mean time I'd love any more guidance you can give against my responses attempting to explain rationale; I am of course happy to cede to your judgement but just want to make sure my reasoning for certain decisions was clear. I will add some more detailed instructions on how to get set up to run the acceptance tests once I can get to my dev environment where I have a working example, but in case you're feeling adventurous in the mean time the high-level is to fiddle with the configs in the acls directory, which has some reasonable documentation. |
@phinze I think I've got everything we discussed in here now. Thanks again for taking the time to give me that feedback. |
@phinze sorry to be a nag... we're about to start a project at work that is intended to make use of this provider to help our applications configure rundeck as an admin tool. If it doesn't get merged into Terraform then we can certainly build it as a local plugin, but I'd like to figure out whether we should just build this plugin from my Terraform branch (if this is likely to get merged eventually) or whether I should spin it up in a separate repo and maintain it there on an ongoing basis. I'm happy to go either way, but the uncertainty is making me anxious. 😁 |
@apparentlymart sorry for the bursty attention from me of late, juggling a couple of projects here, and meanwhile you've just been consistently putting out solid work on Terraform! Giving this another once over and then I'll land it. 👍 |
Thanks @phinze! This is much appreciated as we're literally just starting on Monday our project that will use this. 😄 (obv. we'll be building from master for right now, but nice to know it's headed towards an official release.) |
As a new terraform and rundeck user, I'm looking forward to this. Is there a projected release number? |
@spuder the provider will come out with the next release, which we don't have officially scheduled yet, but - as a guess - should be cut within the next few weeks. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Rundeck is a job scheduler and runbook automation tool. Integrating it into Terraform is useful for a number of reasons:
In order to support the above use-cases, I'm concentrating on the following Rundeck resources to start:
I don't plan to solve for the Rundeck provisioning usecase in this PR, but will probably attack that one in a separate PR once I'm done with this one.