[logs][archives] Add Resource #544
Conversation
ghost
added
the
txominpelu
force-pushed
the
inigo.mediavilla/logs_archives_provider
branch
from
f80b036 to
3b4779d
Compare
|
Hi, Thanks for this PR. Any reasons why we have to use your mocking system instead of the one already in place ? |
txominpelu
force-pushed
the
inigo.mediavilla/logs_archives_provider
branch
4 times, most recently
from
96dd85d to
3f4dd0e
Compare
|
|
||
| func buildAzureMap(destination datadogV2.LogsArchiveDestinationAzure) map[string]interface{} { | ||
| result := make(map[string]interface{}) | ||
| result["client_id"] = destination.Integration.ClientId |
There was a problem hiding this comment.
Could we use Getter instead? (here and similar places)
| } | ||
|
|
||
| //Map to model | ||
| func buildDatadogArchiveCreateReq(d *schema.ResourceData) (datadogV2.LogsArchiveCreateRequest, error) { |
There was a problem hiding this comment.
Let's return pointers whenever possible
| } | ||
|
|
||
| func buildCreateReqDestination(d *schema.ResourceData) (datadogV2.LogsArchiveCreateRequestDestination, error) { | ||
| emptyDestination := datadogV2.LogsArchiveCreateRequestDestination{} |
There was a problem hiding this comment.
Would it be possible to use datadogV2.New... methods instead (here and similar places)
| return defined | ||
| } | ||
|
|
||
| var buildCreateReqDestinationByTypeFunctions = map[string]func(map[string]interface{}) (datadogV2.LogsArchiveCreateRequestDestination, error){ |
There was a problem hiding this comment.
This variable is only used in one place. Do we need to keep this as a variable?
There was a problem hiding this comment.
I agree, we don't need it. I'll move that back to the only place where it is called.
| if !ok { | ||
| return &datadogV2.LogsArchiveDestinationAzure{}, fmt.Errorf("tenant_id is not defined") | ||
| } | ||
| integration := datadogV2.LogsArchiveIntegrationAzure{ |
There was a problem hiding this comment.
Let try to use New methods as much as possible
| httpClient := &http.Client{Transport: logging.NewTransport("Datadog", rec)} | ||
| datadogClientV1 := buildDatadogClientV1(httpClient) | ||
| datadogClientV1.GetConfig().Debug = true | ||
| authV1, err := buildAuthV1(os.Getenv("DD_API_KEY"), os.Getenv("DD_APP_KEY"), os.Getenv("DD_HOST")) | ||
| if err != nil { | ||
| t.Fatalf("Error creating Datadog Client context: %s", err) | ||
| } | ||
| var testAzureAcct = datadogV1.AzureAccount{ | ||
| ClientId: datadogV1.PtrString("testc7f6-1234-5678-9101-3fcbf464test"), | ||
| ClientSecret: datadogV1.PtrString("testingx./Sw*g/Y33t..R1cH+hScMDt"), | ||
| TenantName: datadogV1.PtrString("my-tenant-id"), | ||
| } | ||
| _, _, err = datadogClientV1.AzureIntegrationApi.CreateAzureIntegration(authV1).Body(testAzureAcct).Execute() | ||
| if err != nil { | ||
| t.Fatalf("Error creating Azure Account: Response %s: %v", err.(datadogV1.GenericOpenAPIError).Body(), err) | ||
| } | ||
| defer deleteAzureIntegration(t, datadogClientV1, authV1, testAzureAcct) | ||
|
|
||
| accProviders := testAccProvidersWithHttpClient(t, httpClient) | ||
| accProvider := testAccProvider(t, accProviders) |
There was a problem hiding this comment.
Let's add comments here in order to explain what we are doing here and why
| } | ||
|
|
||
| // readFixture opens the file at path and returns the contents as a string | ||
| func readFixture(t *testing.T, path string) string { |
There was a problem hiding this comment.
Do we still need fixture now?
There was a problem hiding this comment.
You're right, this is no longer required. I will clean it up.
| func buildAzureDestination(d map[string]interface{}) (*datadogV2.LogsArchiveDestinationAzure, error) { | ||
| clientId, ok := d["client_id"] | ||
| if !ok { | ||
| return &datadogV2.LogsArchiveDestinationAzure{}, fmt.Errorf("client_id is not defined") |
There was a problem hiding this comment.
should we just return nil instead of an object here and below?
| func buildGCSDestination(d map[string]interface{}) (*datadogV2.LogsArchiveDestinationGCS, error) { | ||
| clientEmail, ok := d["client_email"] | ||
| if !ok { | ||
| return &datadogV2.LogsArchiveDestinationGCS{}, fmt.Errorf("client_email is not defined") |
| rec := initRecorder(t) | ||
| defer rec.Stop() | ||
| httpClient := &http.Client{Transport: logging.NewTransport("Datadog", rec)} | ||
| // At the moment there's no azure integration in tf so we manually: |
There was a problem hiding this comment.
Should we wait for https://github.com/terraform-providers/terraform-provider-datadog/pull/556 ?
| name = "my first gcs archive" | ||
| query = "service:tata" | ||
| gcs = { | ||
| bucket = "dd-logs-test-datadog-api-client-go" |
There was a problem hiding this comment.
Let's clean the indentation
| name = "my first s3 archive" | ||
| query = "service:tutu" | ||
| s3 = { | ||
| bucket = "my-bucket" |
| * `client_email` - (Required) Your client email. | ||
| * `project_id` - (Required) Your project id. | ||
| * `azure` - (Optional) Definition of an azure archive. | ||
| storage_account = "storageAccount" |
There was a problem hiding this comment.
Not really, I will fix it.
| name = "my first s3 archive after update" | ||
| query = "service:tutu" | ||
| s3 = { | ||
| bucket = "my-bucket" |
txominpelu
force-pushed
the
inigo.mediavilla/logs_archives_provider
branch
from
878a143 to
1463897
Compare
Related to https://github.com/terraform-providers/terraform-provider-datadog/issues/466