Skip to content

Commit c14aa63

Browse files
author
Sander van Harmelen
committed
Delete all deprecated parameters before the 0.7 release
Updated the docs accordingly and also executed all the acceptance tests after making the changes…
1 parent 57f21b4 commit c14aa63

31 files changed

Lines changed: 319 additions & 1113 deletions

builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go

Lines changed: 29 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cloudstack
22

33
import (
4-
"errors"
54
"fmt"
65
"strconv"
76
"strings"
@@ -22,18 +21,9 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
2221

2322
Schema: map[string]*schema.Schema{
2423
"network_id": &schema.Schema{
25-
Type: schema.TypeString,
26-
Optional: true,
27-
ForceNew: true,
28-
ConflictsWith: []string{"network"},
29-
},
30-
31-
"network": &schema.Schema{
32-
Type: schema.TypeString,
33-
Optional: true,
34-
ForceNew: true,
35-
Deprecated: "Please use the `network_id` field instead",
36-
ConflictsWith: []string{"network_id"},
24+
Type: schema.TypeString,
25+
Required: true,
26+
ForceNew: true,
3727
},
3828

3929
"managed": &schema.Schema{
@@ -49,17 +39,11 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
4939
Schema: map[string]*schema.Schema{
5040
"cidr_list": &schema.Schema{
5141
Type: schema.TypeSet,
52-
Optional: true,
42+
Required: true,
5343
Elem: &schema.Schema{Type: schema.TypeString},
5444
Set: schema.HashString,
5545
},
5646

57-
"source_cidr": &schema.Schema{
58-
Type: schema.TypeString,
59-
Optional: true,
60-
Deprecated: "Please use the `cidr_list` field instead",
61-
},
62-
6347
"protocol": &schema.Schema{
6448
Type: schema.TypeString,
6549
Required: true,
@@ -102,29 +86,13 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
10286
}
10387

10488
func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interface{}) error {
105-
cs := meta.(*cloudstack.CloudStackClient)
106-
10789
// Make sure all required parameters are there
10890
if err := verifyEgressFirewallParams(d); err != nil {
10991
return err
11092
}
11193

112-
network, ok := d.GetOk("network_id")
113-
if !ok {
114-
network, ok = d.GetOk("network")
115-
}
116-
if !ok {
117-
return errors.New("Either `network_id` or [deprecated] `network` must be provided.")
118-
}
119-
120-
// Retrieve the network ID
121-
networkid, e := retrieveID(cs, "network", network.(string))
122-
if e != nil {
123-
return e.Error()
124-
}
125-
12694
// We need to set this upfront in order to be able to save a partial state
127-
d.SetId(networkid)
95+
d.SetId(d.Get("network_id").(string))
12896

12997
// Create all rules that are configured
13098
if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 {
@@ -144,11 +112,7 @@ func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interfa
144112
return resourceCloudStackEgressFirewallRead(d, meta)
145113
}
146114

147-
func createEgressFirewallRules(
148-
d *schema.ResourceData,
149-
meta interface{},
150-
rules *schema.Set,
151-
nrs *schema.Set) error {
115+
func createEgressFirewallRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, nrs *schema.Set) error {
152116
var errs *multierror.Error
153117

154118
var wg sync.WaitGroup
@@ -183,10 +147,7 @@ func createEgressFirewallRules(
183147

184148
return errs.ErrorOrNil()
185149
}
186-
func createEgressFirewallRule(
187-
d *schema.ResourceData,
188-
meta interface{},
189-
rule map[string]interface{}) error {
150+
func createEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
190151
cs := meta.(*cloudstack.CloudStackClient)
191152
uuids := rule["uuids"].(map[string]interface{})
192153

@@ -199,7 +160,11 @@ func createEgressFirewallRule(
199160
p := cs.Firewall.NewCreateEgressFirewallRuleParams(d.Id(), rule["protocol"].(string))
200161

201162
// Set the CIDR list
202-
p.SetCidrlist(retrieveCidrList(rule))
163+
var cidrList []string
164+
for _, cidr := range rule["cidr_list"].(*schema.Set).List() {
165+
cidrList = append(cidrList, cidr.(string))
166+
}
167+
p.SetCidrlist(cidrList)
203168

204169
// If the protocol is ICMP set the needed ICMP parameters
205170
if rule["protocol"].(string) == "icmp" {
@@ -307,11 +272,17 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
307272
// Delete the known rule so only unknown rules remain in the ruleMap
308273
delete(ruleMap, id.(string))
309274

275+
// Create a set with all CIDR's
276+
cidrs := &schema.Set{F: schema.HashString}
277+
for _, cidr := range strings.Split(r.Cidrlist, ",") {
278+
cidrs.Add(cidr)
279+
}
280+
310281
// Update the values
311282
rule["protocol"] = r.Protocol
312283
rule["icmp_type"] = r.Icmptype
313284
rule["icmp_code"] = r.Icmpcode
314-
setCidrList(rule, r.Cidrlist)
285+
rule["cidr_list"] = cidrs
315286
rules.Add(rule)
316287
}
317288

@@ -339,9 +310,15 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
339310
// Delete the known rule so only unknown rules remain in the ruleMap
340311
delete(ruleMap, id.(string))
341312

313+
// Create a set with all CIDR's
314+
cidrs := &schema.Set{F: schema.HashString}
315+
for _, cidr := range strings.Split(r.Cidrlist, ",") {
316+
cidrs.Add(cidr)
317+
}
318+
342319
// Update the values
343320
rule["protocol"] = r.Protocol
344-
setCidrList(rule, r.Cidrlist)
321+
rule["cidr_list"] = cidrs
345322
ports.Add(port)
346323
}
347324

@@ -451,11 +428,7 @@ func resourceCloudStackEgressFirewallDelete(d *schema.ResourceData, meta interfa
451428
return nil
452429
}
453430

454-
func deleteEgressFirewallRules(
455-
d *schema.ResourceData,
456-
meta interface{},
457-
rules *schema.Set,
458-
ors *schema.Set) error {
431+
func deleteEgressFirewallRules(d *schema.ResourceData, meta interface{}, rules *schema.Set, ors *schema.Set) error {
459432
var errs *multierror.Error
460433

461434
var wg sync.WaitGroup
@@ -491,16 +464,13 @@ func deleteEgressFirewallRules(
491464
return errs.ErrorOrNil()
492465
}
493466

494-
func deleteEgressFirewallRule(
495-
d *schema.ResourceData,
496-
meta interface{},
497-
rule map[string]interface{}) error {
467+
func deleteEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
498468
cs := meta.(*cloudstack.CloudStackClient)
499469
uuids := rule["uuids"].(map[string]interface{})
500470

501471
for k, id := range uuids {
502472
// We don't care about the count here, so just continue
503-
if k == "#" {
473+
if k == "%" {
504474
continue
505475
}
506476

@@ -542,17 +512,6 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error {
542512
}
543513

544514
func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
545-
cidrList := rule["cidr_list"].(*schema.Set)
546-
sourceCidr := rule["source_cidr"].(string)
547-
if cidrList.Len() == 0 && sourceCidr == "" {
548-
return fmt.Errorf(
549-
"Parameter cidr_list is a required parameter")
550-
}
551-
if cidrList.Len() > 0 && sourceCidr != "" {
552-
return fmt.Errorf(
553-
"Parameter source_cidr is deprecated and cannot be used together with cidr_list")
554-
}
555-
556515
protocol := rule["protocol"].(string)
557516
if protocol != "tcp" && protocol != "udp" && protocol != "icmp" {
558517
return fmt.Errorf(

builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go

Lines changed: 19 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,15 @@ func TestAccCloudStackEgressFirewall_basic(t *testing.T) {
2323
resource.TestCheckResourceAttr(
2424
"cloudstack_egress_firewall.foo", "network_id", CLOUDSTACK_NETWORK_1),
2525
resource.TestCheckResourceAttr(
26-
"cloudstack_egress_firewall.foo", "rule.#", "2"),
26+
"cloudstack_egress_firewall.foo", "rule.#", "1"),
2727
resource.TestCheckResourceAttr(
2828
"cloudstack_egress_firewall.foo",
29-
"rule.1081385056.cidr_list.3378711023",
29+
"rule.2905891128.cidr_list.3378711023",
3030
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
3131
resource.TestCheckResourceAttr(
32-
"cloudstack_egress_firewall.foo", "rule.1081385056.protocol", "tcp"),
33-
resource.TestCheckResourceAttr(
34-
"cloudstack_egress_firewall.foo", "rule.1081385056.ports.32925333", "8080"),
35-
resource.TestCheckResourceAttr(
36-
"cloudstack_egress_firewall.foo",
37-
"rule.1129999216.source_cidr",
38-
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
32+
"cloudstack_egress_firewall.foo", "rule.2905891128.protocol", "tcp"),
3933
resource.TestCheckResourceAttr(
40-
"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
41-
resource.TestCheckResourceAttr(
42-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
43-
resource.TestCheckResourceAttr(
44-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
34+
"cloudstack_egress_firewall.foo", "rule.2905891128.ports.32925333", "8080"),
4535
),
4636
},
4737
},
@@ -61,25 +51,15 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) {
6151
resource.TestCheckResourceAttr(
6252
"cloudstack_egress_firewall.foo", "network_id", CLOUDSTACK_NETWORK_1),
6353
resource.TestCheckResourceAttr(
64-
"cloudstack_egress_firewall.foo", "rule.#", "2"),
54+
"cloudstack_egress_firewall.foo", "rule.#", "1"),
6555
resource.TestCheckResourceAttr(
6656
"cloudstack_egress_firewall.foo",
67-
"rule.1081385056.cidr_list.3378711023",
57+
"rule.2905891128.cidr_list.3378711023",
6858
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
6959
resource.TestCheckResourceAttr(
70-
"cloudstack_egress_firewall.foo", "rule.1081385056.protocol", "tcp"),
71-
resource.TestCheckResourceAttr(
72-
"cloudstack_egress_firewall.foo", "rule.1081385056.ports.32925333", "8080"),
60+
"cloudstack_egress_firewall.foo", "rule.2905891128.protocol", "tcp"),
7361
resource.TestCheckResourceAttr(
74-
"cloudstack_egress_firewall.foo",
75-
"rule.1129999216.source_cidr",
76-
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
77-
resource.TestCheckResourceAttr(
78-
"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
79-
resource.TestCheckResourceAttr(
80-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
81-
resource.TestCheckResourceAttr(
82-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
62+
"cloudstack_egress_firewall.foo", "rule.2905891128.ports.32925333", "8080"),
8363
),
8464
},
8565

@@ -90,37 +70,27 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) {
9070
resource.TestCheckResourceAttr(
9171
"cloudstack_egress_firewall.foo", "network_id", CLOUDSTACK_NETWORK_1),
9272
resource.TestCheckResourceAttr(
93-
"cloudstack_egress_firewall.foo", "rule.#", "3"),
73+
"cloudstack_egress_firewall.foo", "rule.#", "2"),
9474
resource.TestCheckResourceAttr(
9575
"cloudstack_egress_firewall.foo",
96-
"rule.59731059.cidr_list.1910468234",
76+
"rule.3593527682.cidr_list.1910468234",
9777
CLOUDSTACK_NETWORK_1_IPADDRESS2+"/32"),
9878
resource.TestCheckResourceAttr(
9979
"cloudstack_egress_firewall.foo",
100-
"rule.59731059.cidr_list.3378711023",
80+
"rule.3593527682.cidr_list.3378711023",
10181
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
10282
resource.TestCheckResourceAttr(
103-
"cloudstack_egress_firewall.foo", "rule.59731059.protocol", "tcp"),
83+
"cloudstack_egress_firewall.foo", "rule.3593527682.protocol", "tcp"),
10484
resource.TestCheckResourceAttr(
105-
"cloudstack_egress_firewall.foo", "rule.59731059.ports.32925333", "8080"),
85+
"cloudstack_egress_firewall.foo", "rule.3593527682.ports.32925333", "8080"),
10686
resource.TestCheckResourceAttr(
10787
"cloudstack_egress_firewall.foo",
108-
"rule.1052669680.source_cidr",
88+
"rule.739924765.cidr_list.3378711023",
10989
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
11090
resource.TestCheckResourceAttr(
111-
"cloudstack_egress_firewall.foo", "rule.1052669680.protocol", "tcp"),
91+
"cloudstack_egress_firewall.foo", "rule.739924765.protocol", "tcp"),
11292
resource.TestCheckResourceAttr(
113-
"cloudstack_egress_firewall.foo", "rule.1052669680.ports.3638101695", "443"),
114-
resource.TestCheckResourceAttr(
115-
"cloudstack_egress_firewall.foo",
116-
"rule.1129999216.source_cidr",
117-
CLOUDSTACK_NETWORK_1_IPADDRESS1+"/32"),
118-
resource.TestCheckResourceAttr(
119-
"cloudstack_egress_firewall.foo", "rule.1129999216.protocol", "tcp"),
120-
resource.TestCheckResourceAttr(
121-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1209010669", "1000-2000"),
122-
resource.TestCheckResourceAttr(
123-
"cloudstack_egress_firewall.foo", "rule.1129999216.ports.1889509032", "80"),
93+
"cloudstack_egress_firewall.foo", "rule.739924765.ports.1889509032", "80"),
12494
),
12595
},
12696
},
@@ -139,7 +109,7 @@ func testAccCheckCloudStackEgressFirewallRulesExist(n string) resource.TestCheck
139109
}
140110

141111
for k, id := range rs.Primary.Attributes {
142-
if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") {
112+
if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.%") {
143113
continue
144114
}
145115

@@ -172,7 +142,7 @@ func testAccCheckCloudStackEgressFirewallDestroy(s *terraform.State) error {
172142
}
173143

174144
for k, id := range rs.Primary.Attributes {
175-
if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") {
145+
if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.%") {
176146
continue
177147
}
178148

@@ -195,15 +165,8 @@ resource "cloudstack_egress_firewall" "foo" {
195165
protocol = "tcp"
196166
ports = ["8080"]
197167
}
198-
199-
rule {
200-
source_cidr = "%s/32"
201-
protocol = "tcp"
202-
ports = ["80", "1000-2000"]
203-
}
204168
}`,
205169
CLOUDSTACK_NETWORK_1,
206-
CLOUDSTACK_NETWORK_1_IPADDRESS1,
207170
CLOUDSTACK_NETWORK_1_IPADDRESS1)
208171

209172
var testAccCloudStackEgressFirewall_update = fmt.Sprintf(`
@@ -217,19 +180,12 @@ resource "cloudstack_egress_firewall" "foo" {
217180
}
218181
219182
rule {
220-
source_cidr = "%s/32"
183+
cidr_list = ["%s/32"]
221184
protocol = "tcp"
222185
ports = ["80", "1000-2000"]
223186
}
224-
225-
rule {
226-
source_cidr = "%s/32"
227-
protocol = "tcp"
228-
ports = ["443"]
229-
}
230187
}`,
231188
CLOUDSTACK_NETWORK_1,
232189
CLOUDSTACK_NETWORK_1_IPADDRESS1,
233190
CLOUDSTACK_NETWORK_1_IPADDRESS2,
234-
CLOUDSTACK_NETWORK_1_IPADDRESS1,
235191
CLOUDSTACK_NETWORK_1_IPADDRESS1)

0 commit comments

Comments
 (0)