-
Notifications
You must be signed in to change notification settings - Fork 452
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
Support for Terraforming Fleet Teams #18750
Conversation
This project adds support for terraforming teams in FleetDM. If you have 100+ teams, managing them is is prone to error, mistakes, lost configuration, and general pain. An industry standard tool like terraform can unify this configuration as code. In order to do this, I wrote a terraform provider that on one end talks to the FleetDM api, and on the other end implements an interface for terraform. More information is in the README. A small sample `main.tf` file is supplied.
@@ -0,0 +1,66 @@ | |||
module terraform-provider-fleetdm |
Check warning
Code scanning / Trivy
golang: net/http, x/net/http2: unlimited number of CONTINUATION frames causes DoS Medium
Installed Version: 0.21.0
Vulnerability CVE-2023-45288
Severity: MEDIUM
Fixed Version: 0.23.0
Link: CVE-2023-45288
@yoderme thanks for the PR! I'll reach out to some of the devs to get some feedback for you. |
@yoderme thanks for the the contribution, this is really awesome. I had some initial thoughts that I'd like to just lay out here for open discussion. At first I was wonder why you might opt to redefine a client implementation(rather than pull in a dependency via go.mod) of teams because I think our client already has support for the required APIs (create, read, update, delete), but then I remembered a couple of issues I ran into doing something similar on another project which I will detail below. Here is the project I was working on https://github.com/edwardsb/fleet-lambda-packager/blob/main/main.go#L60 and the client usage. There were a few hurdles getting to this point.
So after painfully remembering all of this I now better understand why it might have been easier just to re-implement the small API required for teams, but it does give us something to think about. We might want to consider properly pulling the The other thing I'd like to bring up was that we were considering pulling the terraform modules into its own repository for a variety of reasons #18191. Which makes me wonder if we should also consider having custom providers live there too? This sounds like another solid +1 for proper go.mod support. Anyways, this was a long-winded way of saying LGTM! |
@edwardsb - The embarrassing answer is that I didn't realize it existed. I saw the API and went from there. I guess I accidentally avoided the issue you ran into. I'm actually a big fan of monorepos, IMHO having multiple repositories wandering around leads to fragmentation, chaos, and means that you may have to re-implement repo-wide policies/tooling. But anyway, I would note that there is "terraform for creating a fleetdm server", and there is "terraform of config in that server" and they are entirely different things. Lastly - what's up with all the failing PR checks, and is there anything I can do to address them? |
Ha! Well that works too! I'd like to use the existing client but we aren't in a position to do so, which is why I think the small re-implementation shouldn't be a blocker.
Generally I am too, but in our case terraform doesn't handle monorepos very well (most of the details are outlined in the issue I linked) but each module we define ends up separately cloning the fleet repo, which turns into dozens of GBs of code in
I think most are due to the fact that the PR is coming from a fork and your fork doesn't have permissions and secrets that are required. |
@edwardsb Thanks for all the comments - so what are my next steps here? Thanks a bunch. |
Someone from the approved code owners needs to approve. @lukeheath @rfairburn when y'all have a chance to check this out and review the discussion so far. |
@yoderme Thank you for your contribution! We are currently in a merge freeze as we prepare for Fleet v4.50.0, but we'll finish reviewing your PR and merge after the freeze is lifted later this week. |
@lukeheath Any news on when this might land? |
@yoderme Apologies for the delay and thank you for following up. We are now out of merge freeze and I will ask for a review from the Go group. |
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.
Looks great! Most of my comments refer to the fleet client helper as I'm not familiar with the implementation of a terraform provider (and the provider's code otherwise looked good to me).
@@ -0,0 +1,66 @@ | |||
module terraform-provider-fleetdm |
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.
Wondering if we should use a more typical github.com/fleetdm/fleet/...
module path here? Not totally sure what the implications are, but I found it weird when I saw the unfamiliar import
syntax for that package. Might be totally ok, just raising in case others have more info on this.
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.
Happy to change it if you like
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 think it's fine, we can always change it later if we see it causes issues.
@mna - thanks for the great comments; addressed basically all your feedback. |
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.
Thank you! The Go code looks good to me!
@@ -0,0 +1,66 @@ | |||
module terraform-provider-fleetdm |
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 think it's fine, we can always change it later if we see it causes issues.
Hey @lukeheath do you think we need to bring a story for this through feature fest? Maybe not? (pulling the It's not a change to the product but increases surface area we commit to maintaining (another way to manage Fleet as code along side Fleet's best practice GitOps) I think up to you. |
FYI @nonpunctual ^ |
I guess since it's community-sourced maybe our ongoing effort to keep it up-to-date is not as high as a product designed & built by Fleet? It's a good point @noahtalerman & it kind of means making a decision about how to maintain external contributions. |
Our recommended best practice for managing teams at scale is Fleet with GitOps. However, GitOps may not work for everyone, so this is a valuable alternative that allows Terraform users not using GitOps to manage teams programatically. @yoderme Thank you for your contribution! |
This project adds support for terraforming teams in Fleet. If you have 100+ teams, managing them is is prone to error, mistakes, lost configuration, and general pain. An industry standard tool like terraform can unify this configuration as code.
In order to do this, I wrote a terraform provider that on one end talks to the Fleet api, and on the other end implements an interface for terraform. More information is in the README.
A small sample
main.tf
file is supplied.