Add resource and data for ovh_dbaas_logs #364

vaxvms · 2023-01-23T11:09:11Z

No description provided.

vaxvms · 2023-01-23T14:27:35Z

ovh/resource_dbaas_logs_cluster.go

+}
+
+func resourceDbaasLogsClusterDelete(d *schema.ResourceData, meta interface{}) error {
+	if false {


What should i do when releasing the logs cluster from terraform management ?
Should i delete all ACL or should i leave them as-is ?
AFAIK, a empty list mean that the cluster has no ACL

When a user execute the command terraform destroy on this resource, he/she should have a cluster like it was before the usage of this resource

What about this scenario :

you order a dedicated cluster

you manage it thru terraform, setting some ACL

you add data in it

you stop managing it thru terraform

As you go back to initial state, you don't have ACL anymore and a cluster full of data anyone can query

jehuty0shift

LGTM

website/docs/r/dbaas_logs_cluster.html.markdown

scraly

Several sugestions made.
Moreover, is it possible to create test acceptances for the new resource?
Thanks :)

scraly · 2023-02-08T13:32:19Z

website/docs/r/dbaas_logs_cluster.html.markdown

+resource "ovh_dbaas_logs_cluster" "ldp" {
+  service_name     = "ldp-xx-xxxxx"
+
+  archive_input_allowed_networks = ["10.0.0.0/16"]


Suggested change

archive_input_allowed_networks = ["10.0.0.0/16"]

archive_allowed_networks = ["10.0.0.0/16"]

website/docs/r/dbaas_logs_cluster.html.markdown

scraly

Acceptance tests are missing for resource_dbaas_logs_cluster resource

scraly

The new acceptance test is not working:

scraly · 2023-02-14T07:05:09Z

After a git pull and relaunch the acceptance test.

First, a message asks me to execute make fmt command to format the code:

$ make testacc TESTARGS="-run TestAccDataSourceDbaasLogs_dedicated"
==> Checking that code complies with gofmt requirements...
gofmt needs running on the following files:
./ovh/resource_dbaas_logs_cluster_test.go
You can use the command: `make fmt` to reformat code.
make: *** [fmtcheck] Error 1

$ git pull
Already up to date.

$ make fmt
gofmt -w $(find . -name '*.go' |grep -v vendor)

And the acceptance test failed:

$ make testacc TESTARGS="-run TestAccDataSourceDbaasLogs_dedicated"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccDataSourceDbaasLogs_dedicated -timeout 240m
?   	github.com/ovh/terraform-provider-ovh	[no test files]
=== RUN   TestAccDataSourceDbaasLogs_dedicated
    data_dbaas_logs_test.go:25: Step 1/1 error: Check failed: Check 1/2 error: data.ovh_dbaas_logs.ldp: Attribute 'cluster_type' expected "DEDICATED", got "PRO"
--- FAIL: TestAccDataSourceDbaasLogs_dedicated (7.86s)
FAIL
FAIL	github.com/ovh/terraform-provider-ovh/ovh	8.216s
?   	github.com/ovh/terraform-provider-ovh/ovh/helpers	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/ovh/terraform-provider-ovh/ovh/helpers/hashcode	(cached) [no tests to run]
FAIL
make: *** [testacc] Error 1

So please execute make fmt in your side and then commit the formatting fix, and please fix the acceptance test :)

Thanks

ovh/data_dbaas_logs_test.go

scraly · 2023-02-15T09:06:55Z

Please define in "sensitive" sensitive informations like certificates, in order to not have the full information during an apply for example :)

This advice is for:

dedicated_input_pem
direct_input_pem

https://developer.hashicorp.com/terraform/tutorials/configuration-language/sensitive-variables

Thanks

website/docs/r/dbaas_logs_cluster.html.markdown

ovh/data_dbaas_logs_test.go

ovh/data_dbaas_logs.go

ovh/resource_dbaas_logs_cluster.go

website/docs/r/dbaas_logs_cluster.html.markdown

ovh/data_dbaas_logs.go

scraly · 2023-02-17T10:16:57Z

ovh/resource_dbaas_logs_cluster_test.go

+}
+
+const testAccDbaasLogsClusterConfig = `
+resource "ovh_dbaas_logs_cluster" "ldp" {


You can add threee more attributes in your resource definition, and three more tests in the acceptance test

resource "ovh_dbaas_logs_cluster" "ldp" { service_name = "%s" archive_allowed_networks = ["10.0.0.0/16"] direct_input_allowed_networks = ["10.0.0.0/16"] query_allowed_networks = ["10.0.0.0/16"] }

and then test if the created query_allowed_networks is equals to ["10.0.0.0/16"] and the same things for the others attributes :)

ovh/provider.go

scraly · 2023-02-17T10:31:24Z

ovh/resource_dbaas_logs_cluster.go

+}
+
+func resourceDbaasLogsClusterDelete(d *schema.ResourceData, meta interface{}) error {
+	if false {


When a user execute the command terraform destroy on this resource, he/she should have a cluster like it was before the usage of this resource

scraly · 2023-02-20T09:42:51Z

website/docs/r/dbaas_logs_cluster.html.markdown

+Reference a DBaaS logs cluster to manipulate ACL on a `DEDICATED` cluster
+type.
+
+As LDP cluster can't be created through API, create and delete


Suggested change

As LDP cluster can't be created through API, create and delete

!> An LDP cluster can't be created and deleted via Terraform at this time. So when Terraform destroys the resource, it only actually restores it.

scraly · 2023-02-20T09:43:00Z

website/docs/r/dbaas_logs_cluster.html.markdown

+type.
+
+As LDP cluster can't be created through API, create and delete
+operation are no-op.


Suggested change

operation are no-op.

scraly · 2023-02-22T08:00:12Z

As we can't, easily, send a warning message to a user when he or she wants to delete/destroy a LDP cluster in order to alert he or she that the cluster will "only" be resetted, I created a feature request: #385

scraly

Approved and need to be improved in the future with a fully create and delete feature

vaxvms commented Jan 23, 2023

View reviewed changes

jehuty0shift approved these changes Jan 31, 2023

View reviewed changes

scraly requested changes Feb 8, 2023

View reviewed changes

website/docs/r/dbaas_logs_cluster.html.markdown Outdated Show resolved Hide resolved

vaxvms force-pushed the ovh_dbaas_logs branch from 2da1709 to e20ff86 Compare February 8, 2023 10:58

scraly requested changes Feb 8, 2023

View reviewed changes

website/docs/r/dbaas_logs_cluster.html.markdown Show resolved Hide resolved

website/docs/r/dbaas_logs_cluster.html.markdown Outdated Show resolved Hide resolved

vaxvms force-pushed the ovh_dbaas_logs branch from e20ff86 to e559854 Compare February 8, 2023 12:44

scraly requested changes Feb 8, 2023

View reviewed changes

vaxvms force-pushed the ovh_dbaas_logs branch from e559854 to 7cffe18 Compare February 8, 2023 15:28

scraly requested changes Feb 13, 2023

View reviewed changes

vaxvms force-pushed the ovh_dbaas_logs branch from 7cffe18 to f274f66 Compare February 13, 2023 13:07

scraly requested changes Feb 13, 2023

View reviewed changes

vaxvms force-pushed the ovh_dbaas_logs branch from f274f66 to 8d0e245 Compare February 13, 2023 16:11

vaxvms force-pushed the ovh_dbaas_logs branch from 8d0e245 to c3a7b3e Compare February 14, 2023 10:11

scraly requested changes Feb 15, 2023

View reviewed changes

ovh/data_dbaas_logs_test.go Outdated Show resolved Hide resolved

scraly requested changes Feb 15, 2023

View reviewed changes

website/docs/r/dbaas_logs_cluster.html.markdown Show resolved Hide resolved

scraly requested changes Feb 15, 2023

View reviewed changes

ovh/data_dbaas_logs_test.go Outdated Show resolved Hide resolved

ovh/data_dbaas_logs_test.go Outdated Show resolved Hide resolved

vaxvms force-pushed the ovh_dbaas_logs branch 3 times, most recently from 9935361 to 97538d6 Compare February 15, 2023 10:47

scraly requested changes Feb 15, 2023

View reviewed changes

ovh/data_dbaas_logs.go Outdated Show resolved Hide resolved

ovh/resource_dbaas_logs_cluster.go Outdated Show resolved Hide resolved

ovh/resource_dbaas_logs_cluster.go Show resolved Hide resolved

website/docs/r/dbaas_logs_cluster.html.markdown Show resolved Hide resolved

vaxvms force-pushed the ovh_dbaas_logs branch 2 times, most recently from 24c8952 to 095e85a Compare February 16, 2023 12:59

scraly requested changes Feb 17, 2023

View reviewed changes

ovh/data_dbaas_logs.go Outdated Show resolved Hide resolved

ovh/data_dbaas_logs.go Outdated Show resolved Hide resolved

scraly requested changes Feb 17, 2023

View reviewed changes

ovh/provider.go Outdated Show resolved Hide resolved

scraly requested changes Feb 17, 2023

View reviewed changes

vaxvms force-pushed the ovh_dbaas_logs branch from 095e85a to 5b2c6a7 Compare February 17, 2023 15:57

scraly requested changes Feb 20, 2023

View reviewed changes

Add resource and data for ovh_dbaas_logs_cluster

f43ec9d

vaxvms force-pushed the ovh_dbaas_logs branch from 5b2c6a7 to f43ec9d Compare February 21, 2023 08:36

scraly mentioned this pull request Feb 22, 2023

[FEATURE] Allows the possibility to return warning message to the user #385

Open

scraly approved these changes Feb 22, 2023

View reviewed changes

scraly added the 0.28.0 label Feb 22, 2023

scraly merged commit 8f53cc1 into ovh:master Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resource and data for ovh_dbaas_logs #364

Add resource and data for ovh_dbaas_logs #364

vaxvms commented Jan 23, 2023

vaxvms Jan 23, 2023

scraly Feb 17, 2023

vaxvms Feb 17, 2023

jehuty0shift left a comment

scraly left a comment

scraly Feb 8, 2023

scraly left a comment

scraly left a comment

scraly commented Feb 14, 2023

scraly commented Feb 15, 2023

scraly Feb 17, 2023

scraly Feb 17, 2023

scraly Feb 20, 2023

scraly Feb 20, 2023

scraly commented Feb 22, 2023

scraly left a comment

	archive_input_allowed_networks = ["10.0.0.0/16"]
	archive_allowed_networks = ["10.0.0.0/16"]

	As LDP cluster can't be created through API, create and delete
	!> An LDP cluster can't be created and deleted via Terraform at this time. So when Terraform destroys the resource, it only actually restores it.

Add resource and data for ovh_dbaas_logs #364

Add resource and data for ovh_dbaas_logs #364

Conversation

vaxvms commented Jan 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jehuty0shift left a comment

Choose a reason for hiding this comment

scraly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scraly left a comment

Choose a reason for hiding this comment

scraly left a comment

Choose a reason for hiding this comment

scraly commented Feb 14, 2023

scraly commented Feb 15, 2023

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

scraly commented Feb 22, 2023

scraly left a comment

Choose a reason for hiding this comment