Add data source load balancer pools #1228

wayt · 2021-09-30T15:32:14Z

This adds a Data Source for Load Balancer Pools resources.

i.e:

data "cloudflare_load_balancer_pools" "pools" {
}

Since there is no actual filter on the API client, the data source doesn't offer any either.

⚠️ this is my first time contributing here, let me know if I missed any guidelines.

cloudflare/data_source_load_balancer_pools_test.go

wayt · 2021-09-30T15:34:19Z

cloudflare/data_source_load_balancer_pools.go

+				Optional: true,
+				Computed: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{


I hesitated to directly use resourceCloudflareLoadBalancerPool().Schema here, although it doesn't offer the id field.

Let me know if you think otherwise.

why not append the id if you need it?

func dataSourceLoadBalancerPoolSchema() map[string]*schema.Schema { schema := resourceCloudflareLoadBalancerPool().Schema schema["id"] = *schema.Schema{ Type: schema.TypeString, Computed: true, } return schema } func dataSourceCloudflareLoadBalancerPools() *schema.Resource { return &schema.Resource{ Read: dataSourceCloudflareLoadBalancerPoolsRead, Schema: dataSourceLoadBalancerPoolSchema(), } }

to be honest though, i'm not sure why you'd need anything other than the id on it's own. what is your use case for having everything you've listed? 🤷‍♂️ from me on the rest of them as i personally wouldn't reference them anywhere.

actually, no, you don't want the resourceCloudflareLoadBalancerPool schema as the fields won't line up correctly as a data source 🤦‍♂️ it's geared towards being a managed resource as opposed to a data source.

😓 So we leave it as is?

what is your use case for having everything you've listed?

We manage pools outside of Terraform (because of automation). But load balancers are in Terraform, so to avoid Terraform wiping them away we need that kind of data source.

We do some other filtering later within Terraform, unfortunately, that's not possible with the API.

tbh that would be weird having a data source that does not provide all the information about a pool; different usecases miight need different information from the pools, so might as well have the complete data source right away! (even if it's bound to evolve with lb pools evolutions)

i guess it really comes down to what you're using it for. the API itself only references IDs so things like the other properties aren't able to be used as references anywhere but i'm not phased if you want to include everything else here.

jacobbednarz · 2021-10-01T04:37:55Z

thanks for solid start here @wayt, it's good! i've got some documentation i'm working on locally for PR minimums but for now, here is the tl;dr for this PR

website documentation is required (see other data source documentation for examples)
changelog note (see changelog-process)
passing tests which you've almost got here

wayt · 2021-10-01T14:42:49Z

There we go, let me know if I missed anything @jacobbednarz

XaF · 2021-10-01T15:11:38Z

cloudflare/data_source_load_balancer_pools.go

+func dataSourceCloudflareLoadBalancerPoolsRead(d *schema.ResourceData, meta interface{}) error {
+	log.Printf("[DEBUG] Reading Load Balancer Pools")
+	client := meta.(*cloudflare.API)
+
+	poolsObj, err := client.ListLoadBalancerPools(context.Background())
+	if err != nil {
+		return fmt.Errorf("error listing load balancer pools: %w", err)
+	}
+
+	poolIds := make([]string, 0)
+	pools := make([]map[string]interface{}, 0, len(poolsObj))
+	for _, p := range poolsObj {
+		pools = append(pools, map[string]interface{}{
+			"id":                 p.ID,
+			"name":               p.Name,
+			"origins":            flattenLoadBalancerOrigins(d, p.Origins),
+			"enabled":            p.Enabled,
+			"minimum_origins":    p.MinimumOrigins,
+			"latitude":           p.Latitude,
+			"longitude":          p.Longitude,
+			"check_regions":      p.CheckRegions,
+			"description":        p.Description,
+			"monitor":            p.Monitor,
+			"notification_email": p.NotificationEmail,
+			"load_shedding":      flattenLoadBalancerLoadShedding(p.LoadShedding),
+			"created_on":         p.CreatedOn.Format(time.RFC3339Nano),
+			"modified_on":        p.ModifiedOn.Format(time.RFC3339Nano),
+		})
+		poolIds = append(poolIds, p.ID)
+	}
+
+	if err := d.Set("pools", pools); err != nil {
+		return fmt.Errorf("error setting load balaner pools: %w", err)
+	}
+
+	d.SetId(stringListChecksum(poolIds))
+	return nil
+}


Would be great to have filtering handled there too.
Even if not needed for the specific usecase here, I believe that it's kind of important to provide ways to filter the pools, either directly from the API request (if the API allows) or with basic pattern matching directly in the provider. Other data resources have similar approaches, waf groups as an example:

https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/cloudflare/data_source_waf_groups.go#L29-L46

https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/cloudflare/data_source_waf_groups.go#L92-L96

https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/cloudflare/data_source_waf_groups.go#L122-L128

I can add this if @jacobbednarz agrees here.

filtering is definitely a nice to have for targeting a specific resource. say you know the name is "example pool" you can chuck that in and have multiple data sources instead of referencing them directly.

Sorry for the delay, I just added filtering on the name and the corresponding doc.

Let me know what else is missing here.

cloudflare/data_source_load_balancer_pools.go

Co-authored-by: Raphaël Beamonte <[email protected]>

xens · 2022-12-06T13:57:36Z

@jacobbednarz I'd like to help on that PR, what's in your opinion the best way to contribute considering that it's been stalled for >1year?

…d_balancer_pools

github-actions · 2022-12-09T01:16:18Z

changelog detected ✅

jacobbednarz · 2022-12-09T02:49:25Z

@xens i've gone ahead and got this up to speed with the new conventions however, i'll have a play later this week to make sure it's working as expected. feel free to pull it locally and give it a whirl too.

github-actions · 2022-12-14T04:39:01Z

This functionality has been released in v3.30.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Add data source load balancer pools

b8c858b

wayt commented Sep 30, 2021

View reviewed changes

cloudflare/data_source_load_balancer_pools_test.go Outdated Show resolved Hide resolved

wayt commented Sep 30, 2021

View reviewed changes

wayt added 3 commits October 1, 2021 10:32

Add load balancer pools datasource doc

9726de4

Add cloudflare_load_balancer_pools datasource changelog

609d0a6

Improve cloudflare_load_balancer_pools datasource test with resources

059c0b3

XaF reviewed Oct 1, 2021

View reviewed changes

cloudflare/data_source_load_balancer_pools.go Outdated Show resolved Hide resolved

wayt and others added 2 commits October 4, 2021 09:35

Update cloudflare/data_source_load_balancer_pools.go

445d2d3

Co-authored-by: Raphaël Beamonte <[email protected]>

Add name filtering on load balancer pools datasource

12d3099

jacobbednarz added the lifecycle/stale label Jul 26, 2022

jacobbednarz added 4 commits December 9, 2022 12:10

modernise docs

04ba50c

Merge remote-tracking branch 'origin/master' into data-cloudflare_loa…

d08581e

…d_balancer_pools

remove more old stuff

9ad311d

update docs

b345209

jacobbednarz added 2 commits December 9, 2022 13:11

convert to newer documentation

daaeb99

fix test assertion

34af471

jacobbednarz merged commit 78a4322 into cloudflare:master Dec 14, 2022

github-actions bot added this to the v3.30.0 milestone Dec 14, 2022

github-actions bot pushed a commit that referenced this pull request Dec 14, 2022

Update CHANGELOG.md for #1228

e719993

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add data source load balancer pools #1228

Add data source load balancer pools #1228

wayt commented Sep 30, 2021

wayt Sep 30, 2021

jacobbednarz Oct 1, 2021 •

edited

Loading

jacobbednarz Oct 1, 2021

wayt Oct 1, 2021

wayt Oct 1, 2021

XaF Oct 1, 2021

jacobbednarz Oct 12, 2021

jacobbednarz commented Oct 1, 2021

wayt commented Oct 1, 2021

XaF Oct 1, 2021

wayt Oct 4, 2021

jacobbednarz Oct 12, 2021 •

edited

Loading

wayt Oct 27, 2021

xens commented Dec 6, 2022

github-actions bot commented Dec 9, 2022

jacobbednarz commented Dec 9, 2022

github-actions bot commented Dec 14, 2022

Add data source load balancer pools #1228

Add data source load balancer pools #1228

Conversation

wayt commented Sep 30, 2021

Choose a reason for hiding this comment

jacobbednarz Oct 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobbednarz commented Oct 1, 2021

wayt commented Oct 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobbednarz Oct 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xens commented Dec 6, 2022

github-actions bot commented Dec 9, 2022

jacobbednarz commented Dec 9, 2022

github-actions bot commented Dec 14, 2022

jacobbednarz Oct 1, 2021 •

edited

Loading

jacobbednarz Oct 12, 2021 •

edited

Loading