Skip to content
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.

Sign up for GitHub

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

SUMO-245309 Add the terraform support for Azure metric sources #710

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

SUMO-245309 Add the terraform support for Azure metric sources #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5397700
SUMO-245309 Add the terraform support for Azure metric sources
yuting-liu Jan 6, 2025
6c1b4a0
SUMO-245309 Fix formatting
yuting-liu Jan 8, 2025
5d6282b
SUMO-245309 Fix the compile error
yuting-liu Jan 8, 2025
d747310
SUMO-245309 Add azure_tag_filters in terraform resource
yuting-liu Jan 9, 2025
df9b4b0
SUMO-245309 Fix UT related with tag filters
yuting-liu Jan 13, 2025
9d5d99d
SUMO-245309 Add the doc for the new resource sumologic_azure_metrics_…
yuting-liu Jan 13, 2025
c0deeeb
SUMO-245309 Resolve the merge conflicts
yuting-liu Jan 13, 2025
6ad5431
SUMO-245309 Fix the azure metrcis update ut
yuting-liu Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ jobs:
SUMOLOGIC_DATA_FORWARDING_BUCKET: ${{ secrets.SUMOLOGIC_DATA_FORWARDING_BUCKET }}
SUMOLOGIC_DATA_FORWARDING_ROLE_ARN: ${{ secrets.SUMOLOGIC_DATA_FORWARDING_ROLE_ARN }}
SUMOLOGIC_DATA_FORWARDING_AWS_REGION: ${{ secrets.SUMOLOGIC_DATA_FORWARDING_AWS_REGION }}
SUMOLOGIC_TEST_AZURE_TENANT_ID: ${{ secrets.SUMOLOGIC_TEST_AZURE_TENANT_ID }}
SUMOLOGIC_TEST_AZURE_CLIENT_ID: ${{ secrets.SUMOLOGIC_TEST_AZURE_CLIENT_ID }}
SUMOLOGIC_TEST_AZURE_CLIENT_SECRET: ${{ secrets.SUMOLOGIC_TEST_AZURE_CLIENT_SECRET }}

# disable go test timeout. We rely on GitHub action timeout.
run: |
Expand Down
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## X.Y.Z (Unreleased)
* Add new change notes here
* Add support for update method to change state of the fields resource
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this bullet point was accidentally deleted

FEATURES:
* **New Resource:** sumologic_azure_metrics_source (GH-710)

## 3.0.0 (December 09, 2024)
**REMOVALS:**
Expand Down
1 change: 1 addition & 0 deletions sumologic/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func Provider() terraform.ResourceProvider {
"sumologic_rum_source": resourceSumologicRumSource(),
"sumologic_role_v2": resourceSumologicRoleV2(),
"sumologic_azure_event_hub_log_source": resourceSumologicGenericPollingSource(),
"sumologic_azure_metrics_source": resourceSumologicGenericPollingSource(),
},
DataSourcesMap: map[string]*schema.Resource{
"sumologic_cse_log_mapping_vendor_product": dataSourceCSELogMappingVendorAndProduct(),
Expand Down
209 changes: 209 additions & 0 deletions sumologic/resource_sumologic_azure_metrics_source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
package sumologic

import (
"fmt"
"os"
"strconv"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/terraform"
)

func TestAccSumologicAzureMetricsSource_create(t *testing.T) {
var azureMetricsSource PollingSource
var collector Collector
cName, cDescription, cCategory := getRandomizedParams()
sName, sDescription, sCategory := getRandomizedParams()
azureMetricsResourceName := "sumologic_azure_metrics_source.azure"
testTenantId := os.Getenv("SUMOLOGIC_TEST_AZURE_TENANT_ID")
testClientId := os.Getenv("SUMOLOGIC_TEST_AZURE_CLIENT_ID")
testClientSecret := os.Getenv("SUMOLOGIC_TEST_AZURE_CLIENT_SECRET")

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAzureMetricsSourceDestroy,
Steps: []resource.TestStep{
{
Config: testAccSumologicAzureMetricsSourceConfig(cName, cDescription, cCategory, sName, sDescription, sCategory, testTenantId, testClientId, testClientSecret),
Check: resource.ComposeTestCheckFunc(
testAccCheckAzureMetricsSourceExists(azureMetricsResourceName, &azureMetricsSource),
testAccCheckAzureMetricsSourceValues(&azureMetricsSource, sName, sDescription, sCategory),
testAccCheckCollectorExists("sumologic_collector.test", &collector),
testAccCheckCollectorValues(&collector, cName, cDescription, cCategory, "Etc/UTC", ""),
resource.TestCheckResourceAttrSet(azureMetricsResourceName, "id"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "name", sName),
resource.TestCheckResourceAttr(azureMetricsResourceName, "description", sDescription),
resource.TestCheckResourceAttr(azureMetricsResourceName, "category", sCategory),
resource.TestCheckResourceAttr(azureMetricsResourceName, "content_type", "AzureMetrics"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "path.0.type", "AzureMetricsPath"),
),
ExpectNonEmptyPlan: true,
},
},
})
}

func TestAccSumologicAzureMetricsSource_update(t *testing.T) {
var azureMetricsSource PollingSource
cName, cDescription, cCategory := getRandomizedParams()
sName, sDescription, sCategory := getRandomizedParams()
sNameUpdated, sDescriptionUpdated, sCategoryUpdated := getRandomizedParams()
azureMetricsResourceName := "sumologic_azure_metrics_source.azure"
testTenantId := os.Getenv("SUMOLOGIC_TEST_AZURE_TENANT_ID")
testClientId := os.Getenv("SUMOLOGIC_TEST_AZURE_CLIENT_ID")
testClientSecret := os.Getenv("SUMOLOGIC_TEST_AZURE_CLIENT_SECRET")

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAzureMetricsSourceDestroy,
Steps: []resource.TestStep{
{
Config: testAccSumologicAzureMetricsSourceConfig(cName, cDescription, cCategory, sName, sDescription, sCategory, testTenantId, testClientId, testClientSecret),
Check: resource.ComposeTestCheckFunc(
testAccCheckAzureMetricsSourceExists(azureMetricsResourceName, &azureMetricsSource),
testAccCheckAzureMetricsSourceValues(&azureMetricsSource, sName, sDescription, sCategory),
resource.TestCheckResourceAttrSet(azureMetricsResourceName, "id"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "name", sName),
resource.TestCheckResourceAttr(azureMetricsResourceName, "description", sDescription),
resource.TestCheckResourceAttr(azureMetricsResourceName, "category", sCategory),
resource.TestCheckResourceAttr(azureMetricsResourceName, "content_type", "AzureMetrics"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "path.0.type", "AzureMetricsPath"),
),
ExpectNonEmptyPlan: true,
},
{
Config: testAccSumologicAzureMetricsSourceConfig(cName, cDescription, cCategory, sNameUpdated, sDescriptionUpdated, sCategoryUpdated, testTenantId, testClientId, testClientSecret),
Check: resource.ComposeTestCheckFunc(
testAccCheckAzureMetricsSourceExists(azureMetricsResourceName, &azureMetricsSource),
testAccCheckAzureMetricsSourceValues(&azureMetricsSource, sNameUpdated, sDescriptionUpdated, sCategoryUpdated),
resource.TestCheckResourceAttrSet(azureMetricsResourceName, "id"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "name", sNameUpdated),
resource.TestCheckResourceAttr(azureMetricsResourceName, "description", sDescriptionUpdated),
resource.TestCheckResourceAttr(azureMetricsResourceName, "category", sCategoryUpdated),
resource.TestCheckResourceAttr(azureMetricsResourceName, "content_type", "AzureMetrics"),
resource.TestCheckResourceAttr(azureMetricsResourceName, "path.0.type", "AzureMetricsPath"),
),
ExpectNonEmptyPlan: true,
},
},
})

}

func testAccCheckAzureMetricsSourceDestroy(s *terraform.State) error {
client := testAccProvider.Meta().(*Client)
for _, rs := range s.RootModule().Resources {
if rs.Type != "sumologic_azure_metrics_source" {
continue
}
if rs.Primary.ID == "" {
return fmt.Errorf("Azure Event Hub Log Source destruction check: Azure Event Hub Log Source ID is not set")
}
id, err := strconv.Atoi(rs.Primary.ID)
if err != nil {
return fmt.Errorf("Encountered an error: " + err.Error())
}
collectorID, err := strconv.Atoi(rs.Primary.Attributes["collector_id"])
if err != nil {
return fmt.Errorf("Encountered an error: " + err.Error())
}
s, err := client.GetPollingSource(collectorID, id)
if err != nil {
return fmt.Errorf("Encountered an error: " + err.Error())
}
if s != nil {
return fmt.Errorf("Polling Source still exists")
}
}
return nil
}

func testAccCheckAzureMetricsSourceExists(n string, pollingSource *PollingSource) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("not found: %s", n)
}
if rs.Primary.ID == "" {
return fmt.Errorf("Polling Source ID is not set")
}
id, err := strconv.Atoi(rs.Primary.ID)
if err != nil {
return fmt.Errorf("Polling Source id should be int; got %s", rs.Primary.ID)
}
collectorID, err := strconv.Atoi(rs.Primary.Attributes["collector_id"])
if err != nil {
return fmt.Errorf("Encountered an error: " + err.Error())
}
c := testAccProvider.Meta().(*Client)
pollingSourceResp, err := c.GetPollingSource(collectorID, id)
if err != nil {
return err
}
*pollingSource = *pollingSourceResp
return nil
}
}

func testAccCheckAzureMetricsSourceValues(pollingSource *PollingSource, name, description, category string) resource.TestCheckFunc {
return func(s *terraform.State) error {
if pollingSource.Name != name {
return fmt.Errorf("bad name, expected \"%s\", got: %#v", name, pollingSource.Name)
}
if pollingSource.Description != description {
return fmt.Errorf("bad description, expected \"%s\", got: %#v", description, pollingSource.Description)
}
if pollingSource.Category != category {
return fmt.Errorf("bad category, expected \"%s\", got: %#v", category, pollingSource.Category)
}
return nil
}
}

func testAccSumologicAzureMetricsSourceConfig(cName, cDescription, cCategory, sName, sDescription, sCategory, testTenantId, testClientId, testClientSecret string) string {
return fmt.Sprintf(`
resource "sumologic_collector" "test" {
name = "%s"
description = "%s"
category = "%s"
}
resource "sumologic_azure_metrics_source" "azure" {
name = "%s"
description = "%s"
category = "%s"
content_type = "AzureMetrics"
collector_id = "${sumologic_collector.test.id}"

authentication {
type = "AzureClientSecretAuthentication"
tenant_id = "%s"
client_id = "%s"
client_secret = "%s"
}

path {
type = "AzureMetricsPath"
environment = "Azure"
limit_to_namespaces = ["Microsoft.ClassicStorage/storageAccounts"]
azure_tag_filters {
type = "AzureTagFilters"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we require this type inside azure_tag_filters? since you created this azure_tag_filters field specifically for Azure source, it means azure_tag_filters field should only be of type AzureTagFilters, there wouldn't be other types of azure_tag_filters. for this one-to-one mapping, we still require user to specify this type?

Copy link
Contributor Author

@yuting-liu yuting-liu Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We require user to specify type = "AzureTagFilters" explicitly is due to the reason that it will make it easier for the conversion from API json back to terraform resource.

When type is defined in api, the responses body of the CREATE/UPDATE API will also include the type of tag filters. Otherwise, type won't display in responses either. Thus, by checking the type from the response body, we can easily know that we should convert TagFilters field to azure_tag_filters or tag_filters. Thus, it's not required for user clarification, it's for backend conversion convenience.

namespace = "Microsoft.ClassicStorage/storageAccounts"
tags {
name = "test-name-1"
values = ["value1"]
}
tags {
name = "test-name-2"
values = ["value2"]
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the array representation here looks very different that the API layer, in the API layer if I want to specify tags for two different namespaces where each namespace there are two tag names, it would look like this

{
    "tagFilters":[
        {
            "type":"AzureTagFilters",
            "namespace":"Microsoft.Web/namespace1",
            "tags":[
                {
                    "name":"name1",
                    "values":["value1", "value2"]
                },
                {
                    "name":"name2",
                    "values":["value1", "value2"]
                }
            ]
        },
        {
            "type":"AzureTagFilters",
            "namespace":"Microsoft.Web/namespace2",
            "tags":[
                {
                    "name":"name1",
                    "values":["value1", "value2"]
                },
                {
                    "name":"name2",
                    "values":["value1", "value2"]
                }
            ]
        }
    ]
}

but it seems based on your example here, the above API config translated to TF config would look like this:

	azure_tag_filters {
		type = "AzureTagFilters"
		namespace = "Microsoft.Web/namespace1"
		tags {
			name = "name1"
			values = ["value1", "value2"]
		}
		tags {
			name = "name2"
			values = ["value1", "value2"]
		}
	}
	azure_tag_filters {
		type = "AzureTagFilters"
		namespace = "Microsoft.Web/namespace1"
		tags {
			name = "test-name-1"
			values = ["value1", "value2"]
		}
		tags {
			name = "test-name-1"
			values = ["value1", "value2"]
		}
	}

azure_tag_filters keyword is repeated for each array element and same goes for tags, which is not the same in API layer. Is that expected and ideal for what we should use here? I notice that values = ["value1", "value2"] is still following the normal array representation, so you have two different array representation in the same section and I'm not sure if that's a good thing.

also if we keep this, then the naming needs to change, basically these two fields should both change from plural to singular because each represents a single array element. In the normal array representation like values = ["value1", "value2"] you have plural values because all the array elements are under this name

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cloudwatch tagFilters field is what I expected the azure_tag_filters to look like https://registry.terraform.io/providers/SumoLogic/sumologic/latest/docs/resources/polling_source#example-usage

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the way kinesis metric source defined the tag_filters: https://registry.terraform.io/providers/SumoLogic/sumologic/latest/docs/resources/kinesis_metrics_source:

  path {
    type = "KinesisMetricPath"
    tag_filters {
        type = "TagFilters"
        namespace = "All"
        tags = ["k3=v3"]
    }
    tag_filters {
        type = "TagFilters"
        namespace = "AWS/Route53"
        tags = ["k1=v1"]
    }
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm currently suspecting if these two forms are equivalent in terraform world. I can do some experiments around this.

Copy link
Contributor Author

@yuting-liu yuting-liu Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing, the only accepted form for a list of objects, this duplicate tag_filters for each list item is required.
Even for CloudWatch source, it has to use tag filters in this way:

terraform-provider-sumologic/sumologic/resource_sumologic_cloudwatch_source_test.go

Lines 178 to 186 in 69d8e42

tag_filters {
type = "TagFilters"
namespace = "All"
tags = ["k3=v3"]
}
tag_filters {
type = "TagFilters"
namespace = "AWS/Route53"
tags = ["k1=v1"]

The reason why documentation page shows that way is that it defines a local variable that is similar to the format of what we see in API TagFilters

  path {
    type = "CloudWatchPath"
    limit_to_regions = ["us-west-2"]
    limit_to_namespaces = ["AWS/Route53","AWS/S3","customNamespace"]

    dynamic "tag_filters" {
    for_each = local.tagfilters
    content {
      type = tag_filters.value.type
      namespace = tag_filters.value.namespace
      tags = tag_filters.value.tags
    }
  }

It does some extra step using dynamic and foreach to flatten the tag filters.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. any idea why values = ["value1", "value2"] is different?

}

lifecycle {
ignore_changes = [authentication.0.client_secret]
}
}`, cName, cDescription, cCategory, sName, sDescription, sCategory, testTenantId, testClientId, testClientSecret)
}
Loading
Loading