Skip to content

Commit

Permalink
Eliminate the need of having defaults, add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@jlpedrosa
jlpedrosa committed Sep 20, 2023
1 parent 77f7e11 commit 497e327
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 100 deletions.
11 changes: 11 additions & 0 deletions routeros/provider_schema_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,17 @@ var (

return iOld == iNew
}

// AlwaysPresentNotUserProvided is a SupressDiff function that prevents values not provided by users to get updated.
// This is necessary in some system-wide fields that are present regardless if the users provides any values.
// Prevents the need of hardcode values for default values, as those are harder to track over time/versions of
// routeros
AlwaysPresentNotUserProvided = func(k, old, new string, d *schema.ResourceData) bool {
if old != "" && new == "" {
return true
}
return false
}
)

func buildReadFilter(m map[string]interface{}) []string {
Expand Down
148 changes: 74 additions & 74 deletions routeros/resource_ip_firewall_connection_tracking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,30 +48,30 @@ func ResourceIPConnectionTracking() *schema.Resource {
"enabled": {
Type: schema.TypeString,
Optional: true,
Default: "yes",
Description: `Allows to disable or enable connection tracking. Disabling connection tracking will cause several firewall features to stop working.
See the list of affected features. Starting from v6.0rc2 default value is auto. This means that connection tracing is disabled until at least one firewall rule is added.`,
ValidateFunc: ValidationAutoYesNo,
ValidateFunc: ValidationAutoYesNo,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"generic_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10m",
Description: "Timeout for all other connection entries",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "Timeout for all other connection entries",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"icmp_timeout": {
Type: schema.TypeString,
Optional: true,
Description: "ICMP connection timeout",
Default: "10s",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "ICMP connection timeout",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"loose_tcp_tracking": {
Type: schema.TypeBool,
Optional: true,
Default: true,
Description: "Disable picking up already established connections",
Type: schema.TypeString,
Optional: true,
Description: "Disable picking up already established connections",
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"max_entries": {
Type: schema.TypeString,
Expand All @@ -80,94 +80,94 @@ func ResourceIPConnectionTracking() *schema.Resource {
Computed: true,
},
"tcp_close_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_close_wait_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_established_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "1d",
Description: "Time when established TCP connection times out.",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "Time when established TCP connection times out.",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_fin_wait_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_last_ack_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_max_retrans_timeout": {
Type: schema.TypeString,
Optional: true,
// Documentation did contain the default, I'm getting it from the docker image default (7.10)
Default: "5m",
Description: "No documentation",
ValidateFunc: ValidationTime,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_syn_received_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "5s",
Description: "TCP SYN timeout.",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "TCP SYN timeout.",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_syn_sent_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "5s",
Description: "TCP SYN timeout.",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "TCP SYN timeout.",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_time_wait_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"tcp_unacked_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "5m",
Description: "No documentation",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "No documentation",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"total_entries": {
Type: schema.TypeInt,
Computed: true,
Description: "Amount of connections that currently connection table holds.",
},
"udp_stream_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "3m",
Description: "Specifies the timeout of UDP connections that has seen packets in both directions",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "Specifies the timeout of UDP connections that has seen packets in both directions",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
"udp_timeout": {
Type: schema.TypeString,
Optional: true,
Default: "10s",
Description: "Specifies the timeout for UDP connections that have seen packets in one direction",
ValidateFunc: ValidationTime,
Type: schema.TypeString,
Optional: true,
Description: "Specifies the timeout for UDP connections that have seen packets in one direction",
ValidateFunc: ValidationTime,
DiffSuppressFunc: AlwaysPresentNotUserProvided,
},
}
return &schema.Resource{
Expand Down
107 changes: 81 additions & 26 deletions routeros/resource_ip_firewall_connection_tracking_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,64 @@ func TestAccIPConnectionTrackingTest_basic(t *testing.T) {
for _, name := range testNames {
t.Run(name, func(t *testing.T) {
resource.Test(t, resource.TestCase{

PreCheck: func() {
testAccPreCheck(t)
testSetTransportEnv(t, name)
},
ProviderFactories: testAccProviderFactories,
Steps: []resource.TestStep{
// we can set all fields to non default
{
Config: testAccIPConnectionTrackingFullConfig(),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(testIPConnectionTracking, "active_ipv4", "true"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "active_ipv6", "true"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "enabled", "yes"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "generic_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "icmp_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "loose_tcp_tracking", "false"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "max_entries", "419840"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_established_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_fin_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_last_ack_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_max_retrans_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_received_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_sent_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_time_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_unacked_timeout", "3m"),
resource.TestCheckResourceAttrWith(testIPConnectionTracking, "total_entries", connectionsIsInAcceptableRange),
resource.TestCheckResourceAttr(testIPConnectionTracking, "udp_stream_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "udp_timeout", "3m"),
),
},

// Empty resource don't override the settings
{
Config: testAccIPConnectionTrackingConfig(),
Config: testAccIPConnectionTrackingEmptyConfig(),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(testIPConnectionTracking, "active_ipv4", "true"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "active_ipv6", "true"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "enabled", "yes"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "generic_timeout", "10m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "icmp_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "loose_tcp_tracking", "true"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "generic_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "icmp_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "loose_tcp_tracking", "false"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "max_entries", "419840"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_wait_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_established_timeout", "1d"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_fin_wait_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_last_ack_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_max_retrans_timeout", "5m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_received_timeout", "5s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_sent_timeout", "5s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_time_wait_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_unacked_timeout", "5m"),
resource.TestCheckResourceAttrWith(testIPConnectionTracking, "total_entries", func(value string) error {
nConn, err := strconv.Atoi(value)
if err != nil {
return fmt.Errorf("the total_entries was not a number %q", err)
}
if nConn <= 0 || nConn >= 100 {
return errors.New("number of tcp connections (total_entries) does not seem correct")
}
return nil
}),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_close_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_established_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_fin_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_last_ack_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_max_retrans_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_received_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_syn_sent_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_time_wait_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "tcp_unacked_timeout", "3m"),
resource.TestCheckResourceAttrWith(testIPConnectionTracking, "total_entries", connectionsIsInAcceptableRange),
resource.TestCheckResourceAttr(testIPConnectionTracking, "udp_stream_timeout", "3m"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "udp_timeout", "10s"),
resource.TestCheckResourceAttr(testIPConnectionTracking, "udp_timeout", "3m"),
),
},
},
Expand All @@ -61,11 +81,46 @@ func TestAccIPConnectionTrackingTest_basic(t *testing.T) {
}
}

func testAccIPConnectionTrackingConfig() string {
func testAccIPConnectionTrackingEmptyConfig() string {
return providerConfig + `
resource "routeros_firewall_connection_tracking" "data" {
}
`
}

func testAccIPConnectionTrackingFullConfig() string {
return providerConfig + `
resource "routeros_firewall_connection_tracking" "data" {
enabled = "yes"
generic_timeout = "3m"
icmp_timeout = "3m"
loose_tcp_tracking = "false"
tcp_close_timeout = "3m"
tcp_close_wait_timeout = "3m"
tcp_established_timeout = "3m"
tcp_fin_wait_timeout = "3m"
tcp_last_ack_timeout = "3m"
tcp_max_retrans_timeout = "3m"
tcp_syn_received_timeout = "3m"
tcp_syn_sent_timeout = "3m"
tcp_time_wait_timeout = "3m"
tcp_unacked_timeout = "3m"
udp_stream_timeout = "3m"
udp_timeout = "3m"
}
`
}

func connectionsIsInAcceptableRange(value string) error {
nConn, err := strconv.Atoi(value)
if err != nil {
return fmt.Errorf("the total_entries was not a number %q", err)
}
if nConn <= 0 || nConn >= 100 {
return errors.New("number of tcp connections (total_entries) does not seem correct")
}
return nil
}

0 comments on commit 497e327

Please sign in to comment.