[WIP] New Resource: cloudflare_spectrum_app #156

ewilde · 2018-11-17T18:46:49Z

Work in progress this PR is to add a new resource to automate cloudflare spectrum applications

Relates to #155

TODO

Test results

resource cloudflare_spectrum_app

GOROOT=/usr/lib/go-1.10 #gosetup
GOPATH=/home/ewilde/data/go #gosetup
/usr/lib/go-1.10/bin/go test -c -i -o /tmp/___resource_cloudflare_spectrum_app_test_go github.com/terraform-providers/terraform-provider-cloudflare/cloudflare #gosetup
/usr/lib/go-1.10/bin/go tool test2json -t /tmp/___resource_cloudflare_spectrum_app_test_go -test.v -test.run "^TestAccCloudflareSpectrumApplication_Basic|TestAccCloudflareSpectrumApplication_Update|TestAccCloudflareSpectrumApplication_CreateAfterManualDestroy$" #gosetup
=== RUN   TestAccCloudflareSpectrumApplication_Basic
--- PASS: TestAccCloudflareSpectrumApplication_Basic (4.91s)
=== RUN   TestAccCloudflareSpectrumApplication_Update
--- PASS: TestAccCloudflareSpectrumApplication_Update (8.10s)
=== RUN   TestAccCloudflareSpectrumApplication_CreateAfterManualDestroy
--- PASS: TestAccCloudflareSpectrumApplication_CreateAfterManualDestroy (6.77s)
PASS

Process finished with exit code 0

data cloudflare_zone

GOROOT=/usr/lib/go-1.10 #gosetup
GOPATH=/home/ewilde/data/go #gosetup
/usr/lib/go-1.10/bin/go test -c -i -o /tmp/___data_source_zone_test_go github.com/terraform-providers/terraform-provider-cloudflare/cloudflare #gosetup
/usr/lib/go-1.10/bin/go tool test2json -t /tmp/___data_source_zone_test_go -test.v -test.run ^TestAccCloudflareZone$ #gosetup
=== RUN   TestAccCloudflareZone
--- PASS: TestAccCloudflareZone (1.60s)
PASS

Process finished with exit code 0

Import

=== RUN   TestAccCloudflareSpectrumApp_Import
=== PAUSE TestAccCloudflareSpectrumApp_Import
=== CONT  TestAccCloudflareSpectrumApp_Import
--- PASS: TestAccCloudflareSpectrumApp_Import (3.69s)
PASS

Process finished with exit code 0

Website

make website-test

/home/ewilde/data/go/src/github.com/hashicorp/terraform-website/content/scripts/check-links.sh "http://127.0.0.1:4567/docs/providers/cloudflare/"
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/ [30474/30474] -> "index.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/robots.txt [44/44] -> "robots.txt.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/index.html [30474/30474] -> "index.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/d/ip_ranges.html [28144/28144] -> "ip_ranges.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/d/zone.html [28532/28532] -> "zone.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/access_application.html [28666/28666] -> "access_application.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/access_policy.html [32425/32425] -> "access_policy.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/access_rule.html [33161/33161] -> "access_rule.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/account_member.html [28646/28646] -> "account_member.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/custom_pages.html [29692/29692] -> "custom_pages.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/filter.html [29840/29840] -> "filter.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/firewall_rule.html [31002/31002] -> "firewall_rule.html.tmp.tmp" [1]
2018-11-19 14:29:41 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/load_balancer.html [33943/33943] -> "load_balancer.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/load_balancer_monitor.html [31101/31101] -> "load_balancer_monitor.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/load_balancer_pool.html [31987/31987] -> "load_balancer_pool.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/page_rule.html [34666/34666] -> "page_rule.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/rate_limit.html [35649/35649] -> "rate_limit.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/record.html [30345/30345] -> "record.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/spectrum_app.html [30151/30151] -> "spectrum_app.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/waf_rule.html [28514/28514] -> "waf_rule.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/worker_route.html [31257/31257] -> "worker_route.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/worker_script.html [30246/30246] -> "worker_script.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/zone.html [29589/29589] -> "zone.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/zone_lockdown.html [31058/31058] -> "zone_lockdown.html.tmp.tmp" [1]
2018-11-19 14:29:42 URL:http://127.0.0.1:4567/docs/providers/cloudflare/r/zone_settings_override.html [37775/37775] -> "zone_settings_override.html.tmp.tmp" [1]
Found no broken links.

FINISHED --2018-11-19 14:29:42--
Total wall clock time: 0.7s
Downloaded: 25 files, 730K in 0.04s (15.9 MB/s)
tf-website-cloudflare-temp
make[1]: Leaving directory '/home/ewilde/data/go/src/github.com/hashicorp/terraform-website'

cloudflare_specturm_app

data cloudflare_zone

jacobbednarz · 2018-11-17T19:57:38Z

nice! ping me when you're ready and i'll be happy to take a look. thanks!

…

On Sun, 18 Nov 2018 at 05:46, Edward Wilde ***@***.***> wrote: *Work in progress* this PR is to add a new resource to automate cloudflare spectrum applications Relates to #155 <https://github.com/terraform-providers/terraform-provider-cloudflare/issues/155> ------------------------------ You can view, comment on, or merge this pull request online at: https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156 Commit Summary - WIP moving computer File Changes - *M* website/cloudflare.erb <https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156/files#diff-0> (3) - *A* website/docs/r/spectrum_app.html.markdown <https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156/files#diff-1> (54) Patch Links: - https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156.patch - https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/terraform-providers/terraform-provider-cloudflare/pull/156>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARSYq1Nj8hZcsDjmxo548rKBKHxfheeks5uwFmZgaJpZM4YnleO> .

ewilde · 2018-11-19T21:19:01Z

@jacobbednarz this PR is ready for review (failing CI pending merge of cloudflare/cloudflare-go#253)

jacobbednarz

This is an awesome start! Some comments/discussions inline.

jacobbednarz · 2018-11-19T22:04:31Z

cloudflare/data_source_zone.go

+	"regexp"
+)
+
+func dataSourceCloudflareZone() *schema.Resource {


I'm a little uncertain on whether a data source for a zone is needed as I would have expected that should you need the details, you would have defined the resource already.

@jacobbednarz, great question. I should have put some context around this in the PR text. Actually we don't plan to manage the zone resource in the same terraform project. We prefer to delegate permission to that important resource to another project, which is only accessible to a team with a different set of credentials. This is why i thought the datasource would be so useful. It's how we currently manage our route53, using a datasource to lookup the zone_id and then passing that when creating related resources

Very interesting (and sensible!) use case. I'm 👌 with this approach given the context.

Love the idea of Zone data source, in fact I've been asked for the same thing internally (a list of zones currently added to an account), but let's pull this out of this PR and create a separate PR.

1 PR == 1 feature

cloudflare/provider.go

cloudflare/resource_cloudflare_spectrum_app.go

website/docs/r/spectrum_app.html.markdown

website/docs/d/zone.html.md

website/docs/r/spectrum_app.html.markdown

ewilde · 2018-11-20T10:57:41Z

@jacobbednarz thanks for the review, I've hopefully addressed all the comments corrects and marked the conversations as resolved with a comment

jacobbednarz · 2018-11-20T21:55:11Z

cloudflare/data_source_zone.go

+					Type: schema.TypeString,
+				},
+			},
+			"meta": {


How did you determine where to draw the line with the fields here? I noticed there are a few missing from our enterprise accounts and was curious if it was intended. Happy to provide an anonymised output if you don't have visibility into the full listing.

I used the attributes from the resource_cloudflare_zone, these look reasonable for a first attempt

cloudflare/provider.go

cloudflare/import_cloudflare_spectrum_app_test.go

cloudflare/resource_cloudflare_spectrum_application.go

ewilde · 2018-11-21T13:19:50Z

@jacobbednarz thanks for another review 😄 all suggestions should be pushed now

jacobbednarz

This seems fine to me, great job! 🌟

There are a bunch of unrelated changes in this PR but I suspect it's a side effect of you force pushing and not rebasing/merging in the changes without fast forwarding that is causing them to show up in here.

ewilde · 2018-11-22T13:51:58Z

@jacobbednarz I've rebased against master, there shouldn't be anything here that's not part of the PR

Signed-off-by: Edward Wilde <[email protected]>

patryk

Could you add import section in documentation, please?

patryk · 2018-12-04T16:41:41Z

cloudflare/resource_cloudflare_spectrum_application.go

+		zoneName = idAttr[0]
+		applicationID = idAttr[1]
+	} else {
+		return nil, fmt.Errorf("invalid id (\"%s\") specified, should be in format \"zoneName/applicationID\"", d.Id())


Can we use Zone ID instead of name here?

patryk · 2018-12-04T16:45:03Z

cloudflare/resource_cloudflare_spectrum_application_test.go

+					testAccCheckCloudflareSpectrumApplicationIDIsValid(name),
+					resource.TestCheckResourceAttr(name, "protocol", "tcp/22"),
+					resource.TestCheckResourceAttr(name, "origin_direct.#", "1"),
+					resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://120.120.102.10:23"),


I think we should use reserved IP address, instead of random public one.
See: https://en.wikipedia.org/wiki/Reserved_IP_addresses

Suggested change

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://120.120.102.10:23"),

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://192.0.2.1:23"),

patryk · 2018-12-04T16:46:24Z

cloudflare/resource_cloudflare_spectrum_application_test.go

+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudflareSpectrumApplicationExists(name, &spectrumApp),
+					testAccCheckCloudflareSpectrumApplicationIDIsValid(name),
+					resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://120.120.102.10:23"),


We should use reserved IP here.

Suggested change

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://120.120.102.10:23"),

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://192.0.2.1:23"),

patryk · 2018-12-04T16:46:30Z

cloudflare/resource_cloudflare_spectrum_application_test.go

+						}
+						return nil
+					},
+					resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://81.120.102.10:23"),


We should use reserved IP here.

Suggested change

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://81.120.102.10:23"),

resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://192.0.2.2:23"),

patryk · 2018-12-04T16:50:39Z

cloudflare/resource_cloudflare_spectrum_application_test.go

+	"name" = "ssh.${data.cloudflare_zone.test.zone}"
+  }
+
+  origin_direct = ["tcp://81.120.102.10:23"]


Suggested change

origin_direct = ["tcp://81.120.102.10:23"]

origin_direct = ["tcp://192.0.2.2:23"]

patryk · 2018-12-04T16:51:35Z

website/docs/r/spectrum_application.html.markdown

+
+## Argument Reference
+
+* `protocol`  - (Required) The port configuration at Cloudflare’s edge. i.e. `tcp/22`.


Suggested change

* `protocol` - (Required) The port configuration at Cloudflare’s edge. i.e. `tcp/22`.

* `protocol` - (Required) The port configuration at Cloudflare’s edge. e.g. `tcp/22`.

patryk · 2018-12-04T16:51:52Z

website/docs/r/spectrum_application.html.markdown

+
+* `protocol`  - (Required) The port configuration at Cloudflare’s edge. i.e. `tcp/22`.
+* `dns` - (Required) The name and type of DNS record for the Spectrum application. Fields documented below.
+* `origin_direct` - (Optional) A list of destination addresses to the origin. i.e. `tcp://192.0.2.1:22`.


Suggested change

* `origin_direct` - (Optional) A list of destination addresses to the origin. i.e. `tcp://192.0.2.1:22`.

* `origin_direct` - (Optional) A list of destination addresses to the origin. e.g. `tcp://192.0.2.1:22`.

patryk · 2018-12-04T16:52:10Z

website/docs/r/spectrum_application.html.markdown

+* `dns` - (Required) The name and type of DNS record for the Spectrum application. Fields documented below.
+* `origin_direct` - (Optional) A list of destination addresses to the origin. i.e. `tcp://192.0.2.1:22`.
+* `origin_dns` - (Optional) A destination DNS addresses to the origin. Fields documented below.
+* `origin_port` - (Optional) If using `origin_dns` this is a required attribute. Origin port to proxy traffice to i.e. `22`.


Suggested change

* `origin_port` - (Optional) If using `origin_dns` this is a required attribute. Origin port to proxy traffice to i.e. `22`.

* `origin_port` - (Optional) If using `origin_dns` this is a required attribute. Origin port to proxy traffice to e.g. `22`.

patryk · 2018-12-04T16:52:32Z

website/docs/r/spectrum_application.html.markdown

+
+**origin_dns**
+
+* `name` - (Required) Fully qualified domain name of the origin i.e. origin-ssh.example.com.


Suggested change

* `name` - (Required) Fully qualified domain name of the origin i.e. origin-ssh.example.com.

* `name` - (Required) Fully qualified domain name of the origin e.g. origin-ssh.example.com.

patryk · 2019-01-11T16:38:41Z

Thanks @ewilde !

ewilde · 2019-01-11T17:05:51Z

@patryk thanks for the review and merge, but think the tests will need updating to use the plural zones data source, this pr was still using the single zone data source to bootstrap some of the integration tests

ewilde · 2019-01-11T17:09:36Z

ill do this now

patryk · 2019-01-11T17:28:33Z

Oh yes, please do.

* The upcoming Purge by Host functionality has been added to the library * flarectl now has a `purge` sub-command for purging from cache. e.g ``` # Purge by host (upcoming) flarectl zone purge --zone=example.com --hosts="repeater.example.com" # Purge everything flarectl zone purge --zone=example.com --everything # Purge by filename flarectl zone purge --zone=example.com \ --files="http://repeater.example.com/styles.css,http://repeater.example.com/javascript.js" ```

ghost added the size/S label Nov 17, 2018

ewilde closed this Nov 18, 2018

ewilde force-pushed the ewilde-spectrum branch from bfa4322 to cfa0ec2 Compare November 18, 2018 12:23

ghost added size/XS and removed size/S labels Nov 18, 2018

ewilde reopened this Nov 18, 2018

ghost added size/XL and removed size/XS labels Nov 19, 2018

ewilde force-pushed the ewilde-spectrum branch from e8a9b45 to bb8e2dd Compare November 19, 2018 15:29

ghost added size/XXL and removed size/XL labels Nov 19, 2018

ewilde mentioned this pull request Nov 19, 2018

Correctly support origin_dns in spectrum application cloudflare/cloudflare-go#252

Closed

ewilde changed the title ~~[WIP] New Resource: cloudflare_spectrum_app~~ New Resource: cloudflare_spectrum_app Nov 19, 2018

jacobbednarz requested changes Nov 19, 2018

View reviewed changes

ewilde force-pushed the ewilde-spectrum branch from bb8e2dd to ab0f712 Compare November 20, 2018 10:52

jacobbednarz reviewed Nov 20, 2018

View reviewed changes

cloudflare/provider.go Outdated Show resolved Hide resolved

jacobbednarz reviewed Nov 20, 2018

View reviewed changes

cloudflare/import_cloudflare_spectrum_app_test.go Outdated Show resolved Hide resolved

jacobbednarz reviewed Nov 20, 2018

View reviewed changes

cloudflare/resource_cloudflare_spectrum_application.go Outdated Show resolved Hide resolved

ewilde force-pushed the ewilde-spectrum branch from ab0f712 to 9648cba Compare November 21, 2018 13:19

jacobbednarz approved these changes Nov 21, 2018

View reviewed changes

ewilde force-pushed the ewilde-spectrum branch from 9648cba to 1e3ca9e Compare November 22, 2018 11:25

ghost added size/XL and removed size/XXL labels Nov 22, 2018

ewilde changed the title ~~New Resource: cloudflare_spectrum_app~~ [WIP] - New Resource: cloudflare_spectrum_app Nov 22, 2018

ewilde force-pushed the ewilde-spectrum branch from 1e3ca9e to 1046198 Compare November 22, 2018 16:50

ewilde changed the title ~~[WIP] - New Resource: cloudflare_spectrum_app~~ New Resource: cloudflare_spectrum_app Nov 22, 2018

ewilde changed the title ~~New Resource: cloudflare_spectrum_app~~ [WIP] New Resource: cloudflare_spectrum_app Nov 23, 2018

New Resource: cloudflare_spectrum_app

b9684be

Signed-off-by: Edward Wilde <[email protected]>

ewilde force-pushed the ewilde-spectrum branch from 1046198 to b9684be Compare November 23, 2018 14:14

patryk requested changes Dec 4, 2018

View reviewed changes

patryk self-assigned this Jan 11, 2019

Changes from PR review

c6610e4

ghost added the kind/documentation Categorizes issue or PR as related to documentation. label Jan 11, 2019

patryk approved these changes Jan 11, 2019

View reviewed changes

deuill approved these changes Jan 11, 2019

View reviewed changes

patryk merged commit 2c7bd43 into cloudflare:master Jan 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] New Resource: cloudflare_spectrum_app #156

[WIP] New Resource: cloudflare_spectrum_app #156

ewilde commented Nov 17, 2018 •

edited

Loading

jacobbednarz commented Nov 17, 2018 via email

ewilde commented Nov 19, 2018

jacobbednarz left a comment

jacobbednarz Nov 19, 2018

ewilde Nov 20, 2018 •

edited

Loading

jacobbednarz Nov 20, 2018

patryk Nov 22, 2018

ewilde commented Nov 20, 2018

jacobbednarz Nov 20, 2018

ewilde Nov 21, 2018

ewilde commented Nov 21, 2018

jacobbednarz left a comment

ewilde commented Nov 22, 2018

patryk left a comment

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk Dec 4, 2018

patryk commented Jan 11, 2019

ewilde commented Jan 11, 2019

ewilde commented Jan 11, 2019

patryk commented Jan 11, 2019

	resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://120.120.102.10:23"),
	resource.TestCheckResourceAttr(name, "origin_direct.0", "tcp://192.0.2.1:23"),

	origin_direct = ["tcp://81.120.102.10:23"]
	origin_direct = ["tcp://192.0.2.2:23"]


		## Argument Reference

		* `protocol` - (Required) The port configuration at Cloudflare’s edge. i.e. `tcp/22`.

	* `origin_direct` - (Optional) A list of destination addresses to the origin. i.e. `tcp://192.0.2.1:22`.
	* `origin_direct` - (Optional) A list of destination addresses to the origin. e.g. `tcp://192.0.2.1:22`.

	* `origin_port` - (Optional) If using `origin_dns` this is a required attribute. Origin port to proxy traffice to i.e. `22`.
	* `origin_port` - (Optional) If using `origin_dns` this is a required attribute. Origin port to proxy traffice to e.g. `22`.


		origin_dns

		* `name` - (Required) Fully qualified domain name of the origin i.e. origin-ssh.example.com.

[WIP] New Resource: cloudflare_spectrum_app #156

[WIP] New Resource: cloudflare_spectrum_app #156

Conversation

ewilde commented Nov 17, 2018 • edited Loading

TODO

Test results

resource cloudflare_spectrum_app

data cloudflare_zone

Import

Website

cloudflare_specturm_app

data cloudflare_zone

jacobbednarz commented Nov 17, 2018 via email

ewilde commented Nov 19, 2018

jacobbednarz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilde Nov 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilde commented Nov 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilde commented Nov 21, 2018

jacobbednarz left a comment

Choose a reason for hiding this comment

ewilde commented Nov 22, 2018

patryk left a 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

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

patryk commented Jan 11, 2019

ewilde commented Jan 11, 2019

ewilde commented Jan 11, 2019

patryk commented Jan 11, 2019

ewilde commented Nov 17, 2018 •

edited

Loading

ewilde Nov 20, 2018 •

edited

Loading