Skip to content

Commit

Permalink
fix(ip_filter): suppress ipv6 default value
Browse files Browse the repository at this point in the history
  • Loading branch information
byashimov committed Feb 11, 2025
1 parent f0017ae commit 5ad1f49
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 76 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ nav_order: 1
<!-- Always keep the following header in place: -->
<!--## [MAJOR.MINOR.PATCH] - YYYY-MM-DD -->

## [MAJOR.MINOR.PATCH] - YYYY-MM-DD

- Fix `ip_filter`, suppress plan for the new `ip_filter` default value (`::/0`) when a new service is created

## [4.34.1] - 2025-02-07

- Fix `organization_user_group` import

## [4.34.0] - 2025-01-29
Expand Down
14 changes: 0 additions & 14 deletions internal/schemautil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package schemautil

import (
"context"
"errors"
"fmt"
"strings"

Expand All @@ -11,7 +10,6 @@ import (
"github.com/aiven/go-client-codegen/handler/service"
"github.com/docker/go-units"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)
Expand Down Expand Up @@ -192,15 +190,3 @@ func StringToDiagWarning(msg string) diag.Diagnostics {
func ErrorToDiagWarning(err error) diag.Diagnostics {
return StringToDiagWarning(err.Error())
}

// ErrorFromDiagnostics converts diag.Diagnostics to error.
// Warning: ignores diag.Warning level diagnostics.
func ErrorFromDiagnostics(diags diag.Diagnostics) error {
var err error
for _, v := range diags {
if v.Severity == diag.Error {
err = multierror.Append(err, errors.New(v.Summary))
}
}
return err
}
17 changes: 5 additions & 12 deletions internal/schemautil/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/samber/lo"

"github.com/aiven/terraform-provider-aiven/internal/common"
"github.com/aiven/terraform-provider-aiven/internal/sdkprovider/userconfig/diff"
)

const (
Expand Down Expand Up @@ -203,20 +204,12 @@ func grafanaReady(s *service.ServiceGetOut) bool {
}

// if IP filter is anything but 0.0.0.0/0 skip Grafana service availability checks
ipFilters, ok := s.UserConfig["ip_filter"]
if ok {
f := ipFilters.([]interface{})
if len(f) > 1 {
log.Printf("[DEBUG] grafana serivce has `%+v` ip filters, and availability checks will be skipped", ipFilters)
if ipFilters, ok := s.UserConfig["ip_filter"]; ok {
list, ok := ipFilters.([]any)
if !(ok && diff.DefaultIPFilters(list...)) {
log.Printf("[DEBUG] grafana serivce has `%v` ip filters, and availability checks will be skipped", ipFilters)
return true
}

if len(f) == 1 {
if f[0] != "0.0.0.0/0" {
log.Printf("[DEBUG] grafana serivce has `%+v` ip filters, and availability checks will be skipped", ipFilters)
return true
}
}
}

var publicGrafana string
Expand Down
8 changes: 4 additions & 4 deletions internal/sdkprovider/service/grafana/grafana_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestAccAiven_grafana_user_config(t *testing.T) {
acc.TestAccCheckAivenServiceCommonAttributes("data.aiven_grafana.common"),
testAccCheckAivenServiceGrafanaAttributes("data.aiven_grafana.common"),
resource.TestCheckResourceAttr(resourceName, "service_name", fmt.Sprintf("test-acc-sr-%s", rName)),
resource.TestCheckResourceAttr(resourceName, "grafana_user_config.0.ip_filter.#", "1"),
resource.TestCheckResourceAttr(resourceName, "grafana_user_config.0.ip_filter.#", "2"),
resource.TestCheckResourceAttr(resourceName, "grafana_user_config.0.alerting_enabled", "true"),
resource.TestCheckResourceAttr(resourceName, "grafana_user_config.0.public_access.0.grafana", "true"),
),
Expand Down Expand Up @@ -243,7 +243,7 @@ resource "aiven_grafana" "bar" {
}
grafana_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
alerting_enabled = true
public_access {
Expand Down Expand Up @@ -283,7 +283,7 @@ resource "aiven_grafana" "bar" {
}
grafana_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
alerting_enabled = true
public_access {
Expand Down Expand Up @@ -362,7 +362,7 @@ resource "aiven_grafana" "bar" {
maintenance_window_time = "10:00:00"
grafana_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
alerting_enabled = true
public_access {
Expand Down
2 changes: 1 addition & 1 deletion internal/sdkprovider/service/kafka/kafka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func testAccCheckAivenServiceKafkaAttributes(n string) resource.TestCheckFunc {
return fmt.Errorf("expected to get a correct public_access.prometheus from Aiven")
}

if a["kafka_user_config.0.ip_filter.0"] != "0.0.0.0/0" {
if a["kafka_user_config.0.ip_filter.0"] != "0.0.0.0/0" || a["kafka_user_config.0.ip_filter.1"] != "::/0" {
return fmt.Errorf("expected to get a correct ip_filter from Aiven")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ resource "aiven_kafka_mirrormaker" "mm" {
service_name = "test-acc-sr-mm-%[2]s"
kafka_mirrormaker_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
kafka_mirrormaker {
refresh_groups_interval_seconds = 600
Expand Down
2 changes: 1 addition & 1 deletion internal/sdkprovider/service/kafka/mirrormaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ resource "aiven_kafka_mirrormaker" "bar" {
service_name = "test-acc-sr-%s"
kafka_mirrormaker_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
kafka_mirrormaker {
refresh_groups_interval_seconds = 600
Expand Down
4 changes: 2 additions & 2 deletions internal/sdkprovider/service/m3db/m3db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ resource "aiven_grafana" "grafana1" {
service_name = "test-acc-sr-g-%s"
grafana_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
alerting_enabled = true
public_access {
Expand Down Expand Up @@ -303,7 +303,7 @@ resource "aiven_grafana" "grafana1" {
service_name = "test-acc-sr-g-%s"
grafana_user_config {
ip_filter = ["0.0.0.0/0"]
ip_filter = ["0.0.0.0/0", "::/0"]
alerting_enabled = true
public_access {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ func TestAccAivenOrganizationUserGroup_Import(t *testing.T) {
func testAccOrganizationUserGroupImportConfig(orgID, name, groupID string) string {
return fmt.Sprintf(`
resource "aiven_organization_user_group" "import_group" {
organization_id = "%s"
name = "%s"
description = "Imported group"
organization_id = "%s"
name = "%s"
description = "Imported group"
}
import {
id = "%s/%s"
to = aiven_organization_user_group.import_group
id = "%s/%s"
to = aiven_organization_user_group.import_group
}
`, orgID, name, orgID, groupID)
}
68 changes: 40 additions & 28 deletions internal/sdkprovider/userconfig/converters/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,32 @@ func TestRenameAliasesToDto(t *testing.T) {
{
serviceType: "kafka",
name: "keeps original key",
src: `{"ip_filter": ["0.0.0.0/0"]}`,
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
src: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
},
{
serviceType: "kafka",
name: "chooses original out if 3",
src: `{"ip_filter": ["0.0.0.0/0"], "ip_filter_string": [], "ip_filter_object": []}`,
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
src: `{"ip_filter": ["0.0.0.0/0", "::/0"], "ip_filter_string": [], "ip_filter_object": []}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
},
{
serviceType: "kafka",
name: "chooses string out if 3",
src: `{"ip_filter": [], "ip_filter_string": ["0.0.0.0/0"], "ip_filter_object": []}`,
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
src: `{"ip_filter": [], "ip_filter_string": ["0.0.0.0/0", "::/0"], "ip_filter_object": []}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
},
{
serviceType: "kafka",
name: "ignores unknown key",
src: `{"whatever": ["0.0.0.0/0"]}`,
expected: `{"whatever": ["0.0.0.0/0"]}`,
src: `{"whatever": ["0.0.0.0/0", "::/0"]}`,
expected: `{"whatever": ["0.0.0.0/0", "::/0"]}`,
},
{
serviceType: `kafka`,
name: `renames "_string" prefix`,
src: `{"ip_filter_string": ["0.0.0.0/0"]}`,
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
src: `{"ip_filter_string": ["0.0.0.0/0", "::/0"]}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
},
{
serviceType: `kafka`,
Expand Down Expand Up @@ -117,28 +117,34 @@ func TestRenameAliasesToTfo(t *testing.T) {
{
serviceType: "kafka",
name: "keeps original key",
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
dto: `{"ip_filter": ["0.0.0.0/0"]}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
dto: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
tfo: newResourceDataMock(
newResourceDataKV("kafka_user_config.0.ip_filter", []string{"0.0.0.0/0"}),
newResourceDataKV("kafka_user_config.0.ip_filter", []string{"0.0.0.0/0", "::/0"}),
),
},
{
serviceType: "kafka",
name: "uses ip_filter_string",
expected: `{"ip_filter_string": ["0.0.0.0/0"]}`,
dto: `{"ip_filter": ["0.0.0.0/0"]}`,
expected: `{"ip_filter_string": ["0.0.0.0/0", "::/0"]}`,
dto: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
tfo: newResourceDataMock(
newResourceDataKV("kafka_user_config.0.ip_filter_string", []string{"0.0.0.0/0"}),
newResourceDataKV("kafka_user_config.0.ip_filter_string", []string{"0.0.0.0/0", "::/0"}),
),
},
{
serviceType: "kafka",
name: "uses ip_filter_object",
expected: `{"ip_filter_object": [{"network": "0.0.0.0/0"}]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"}]}`,
expected: `{"ip_filter_object": [{"network": "0.0.0.0/0"},{"network": "::/0"}]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"},{"network": "::/0"}]}`,
tfo: newResourceDataMock(
newResourceDataKV("kafka_user_config.0.ip_filter_object", []map[string]string{{"network": "0.0.0.0/0"}}),
newResourceDataKV(
"kafka_user_config.0.ip_filter_object",
[]map[string]string{
{"network": "0.0.0.0/0"},
{"network": "::/0"},
},
),
),
},
{
Expand Down Expand Up @@ -176,28 +182,34 @@ func TestRenameAliasesToTfo(t *testing.T) {
{
serviceType: "thanos",
name: "ip_filter gets a list of objects",
expected: `{"ip_filter": ["0.0.0.0/0"]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"}]}`,
expected: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"},{"network": "::/0"}]}`,
tfo: newResourceDataMock(
newResourceDataKV("thanos_user_config.0.ip_filter", []string{"0.0.0.0/0"}),
newResourceDataKV("thanos_user_config.0.ip_filter", []string{"0.0.0.0/0", "::/0"}),
),
},
{
serviceType: "thanos",
name: "ip_filter_string gets a list of objects",
expected: `{"ip_filter_string": ["0.0.0.0/0"]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"}]}`,
expected: `{"ip_filter_string": ["0.0.0.0/0", "::/0"]}`,
dto: `{"ip_filter": [{"network": "0.0.0.0/0"},{"network": "::/0"}]}`,
tfo: newResourceDataMock(
newResourceDataKV("thanos_user_config.0.ip_filter_string", []string{"0.0.0.0/0"}),
newResourceDataKV("thanos_user_config.0.ip_filter_string", []string{"0.0.0.0/0", "::/0"}),
),
},
{
serviceType: "thanos",
name: "ip_filter_object gets a list of strings",
expected: `{"ip_filter_object": [{"network": "0.0.0.0/0"}]}`,
dto: `{"ip_filter": ["0.0.0.0/0"]}`,
expected: `{"ip_filter_object": [{"network": "0.0.0.0/0"},{"network": "::/0"}]}`,
dto: `{"ip_filter": ["0.0.0.0/0", "::/0"]}`,
tfo: newResourceDataMock(
newResourceDataKV("thanos_user_config.0.ip_filter_object", []map[string]string{{"network": "0.0.0.0/0"}}),
newResourceDataKV(
"thanos_user_config.0.ip_filter_object",
[]map[string]string{
{"network": "0.0.0.0/0"},
{"network": "::/0"},
},
),
),
},
{
Expand Down
60 changes: 52 additions & 8 deletions internal/sdkprovider/userconfig/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package diff

import (
"regexp"
"slices"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -74,14 +75,26 @@ func SuppressUnchanged(k, oldValue, newValue string, d *schema.ResourceData) boo
return false
}

// suppressIPFilterSet ip_filter list has specific logic, like default list value
func suppressIPFilterSet(k, oldValue, newValue string, d *schema.ResourceData) bool {
// Suppresses ip_filter = [0.0.0.0/0]
path := strings.Split(k, ".")
// Turns ~ip_filter.1234 to ~ip_filter.#
v, ok := d.GetOk(strings.Join(path[:len(path)-1], ".") + ".#")
// Literally, if the value is "0.0.0.0/0" and the parent's length is "1"
return oldValue == "0.0.0.0/0" && newValue == "" && ok && v.(int) == 1
// suppressIPFilterSet ip_filter list has specific logic
// Suppresses default value removal, but doesn't suppress an empty ip_filter list:
// - ip_filter=["0.0.0.0/0", "::/0"]
// + ip_filter=[]
// This function is called for ip_filter itself and its every element.
func suppressIPFilterSet(k, _, newValue string, d *schema.ResourceData) bool {
// Turns ~ip_filter.1234 to ~ip_filter to get the set.
const ipFilterDepth = 3 // foo_user_config.0.ip_filter
path := strings.Join(strings.Split(k, ".")[0:ipFilterDepth], ".")
set, ok := d.Get(path).(*schema.Set)
if !ok {
// This can't happen, but we need to avoid panic.
return false
}

// Literally, if the value is removed, and the parent is equal to ["0.0.0.0/0", "::/0"],
// suppress the diff, because it is the default value.
// 1. "" — means that the element is removed, suppress removal only
// 2 proves all values are default.
return newValue == "" && set.Len() == len(defaultIPFilters()) && DefaultIPFilters(set.List()...)
}

// isObjectSet returns true if given k is for collection of objects
Expand All @@ -104,3 +117,34 @@ func isObjectSet(k string, d *schema.ResourceData) bool {
t := value.Type()
return t.IsSetType() && t.ElementType().IsObjectType()
}

func defaultIPFilters() []string {
return []string{"0.0.0.0/0", "::/0"}
}

// DefaultIPFilters validates if the given ip_filter list is default
func DefaultIPFilters(args ...any) bool {
list := defaultIPFilters()
for _, a := range args {
v, ok := NormalizeIPFilter(a)
if !ok || !slices.Contains(list, v) {
return false
}
}
return len(args) > 0
}

// NormalizeIPFilter returns ip_filter network:
// 1. when it is a string, it returns string
// 2. when it is a map, it returns the value of the "network" key
func NormalizeIPFilter(v any) (string, bool) {
s, ok := v.(string)
if ok {
return s, ok
}
m, ok := v.(map[string]any)
if ok {
return NormalizeIPFilter(m["network"])
}
return "", false
}

0 comments on commit 5ad1f49

Please sign in to comment.