-
Notifications
You must be signed in to change notification settings - Fork 44
Inconsistent result when creating cloudstack_firewall #115
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.
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
Comments
My colleague just found this: https://www.reddit.com/r/Terraform/comments/m5nv14/comment/gr29zct/?utm_source=share&utm_medium=web2x&context=3 Using the argument 'managed = true' workaround my problem, but it seems it would be cleaner when the problem is catched from the provider. What do you think? |
Thanks for reporting the issue The issue is occurring only if project are used. Marking it as a bug and improvement request |
Hello, |
Hello. After searching for the source of the issue, I found it here, the issue is related to the condition:
After changing it to:
the firewall was created successfully with the parameters:
However, I'm not sure if this is the correct approach to solving the issue. |
Hey @baltazorbest , nice to meet you. As said please submit your change in a Pull Request and we can test. As you said you have already tested in your production environment, we are more than helf way merging it already ;) . Meybe a unit test to make sure it is covored would be nice. |
@baltazorbest I'm not convinced that swapping the logic of that conditional is the fix we're looking for. I was hitting a similar issue with Network ACL rules, and after some hours of debugging found that it was occurring there because I wasn't supplying a project ID to _, count, err := cs.NetworkACL.GetNetworkACLListByID(
d.Id(),
cloudstack.WithProject(d.Get("project").(string)),
)
if err != nil {
if count == 0 {
log.Printf(
"[DEBUG] Network ACL list %s does no longer exist", d.Id())
d.SetId("")
return nil
}
return err
} func (s *NetworkACLService) GetNetworkACLListByID(id string, opts ...OptionFunc) (*NetworkACLList, int, error) {
p := &ListNetworkACLListsParams{}
p.p = make(map[string]interface{})
p.p["id"] = id
for _, fn := range append(s.cs.options, opts...) {
if err := fn(s.cs, p); err != nil {
return nil, -1, err
}
}
l, err := s.ListNetworkACLLists(p)
if err != nil {
if strings.Contains(err.Error(), fmt.Sprintf(
"Invalid parameter id value=%s due to incorrect long value format, "+
"or entity does not exist", id)) {
return nil, 0, fmt.Errorf("No match found for %s: %+v", id, l)
}
return nil, -1, err
}
// -
// - This is where a zero-count set results in an error, which is cascaded to the calling function
// -
if l.Count == 0 {
return nil, l.Count, fmt.Errorf("No match found for %s: %+v", id, l)
}
if l.Count == 1 {
return l.NetworkACLLists[0], l.Count, nil
}
return nil, l.Count, fmt.Errorf("There is more then one result for NetworkACLList UUID: %s!", id)
}
When I included the In the case of // Get all the rules from the running environment
p := cs.Firewall.NewListFirewallRulesParams()
p.SetIpaddressid(d.Id())
p.SetListall(true)
l, err := cs.Firewall.ListFirewallRules(p)
if err != nil {
return err
} func (s *FirewallService) ListFirewallRules(p *ListFirewallRulesParams) (*ListFirewallRulesResponse, error) {
resp, err := s.cs.newRequest("listFirewallRules", p.toURLValues())
if err != nil {
return nil, err
}
resp, err = convertFirewallServiceResponse(resp)
if err != nil {
return nil, err
}
var r ListFirewallRulesResponse
if err := json.Unmarshal(resp, &r); err != nil {
return nil, err
}
return &r, nil
} In this case, if the firewall is scoped to a project, this code will never find any rules, as the // -
// ruleMap is supposed to be a map of existing rules fetched from the API
// BUT:
// if the firewall belongs to a project, it will always be empty
// This is incorrect behaviour
// -
ruleMap := make(map[string]*cloudstack.FirewallRule, l.Count)
for _, r := range l.FirewallRules {
ruleMap[r.Id] = r
}
// Create an empty schema.Set to hold all rules
rules := resourceCloudStackFirewall().Schema["rule"].ZeroValue().(*schema.Set)
// Read all rules that are configured
if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
for _, rule := range rs.List() {
// ...rule matching logic Under normal circumstances,
This logic exists because a managed If the firewall is scoped to a project, and // If we've matched any of the existing Cloudstack firewall rules to the rules in the resource,
// include them in the resulting ResourceData:
if rules.Len() > 0 {
d.Set("rule", rules)
// If we haven't, AND this resource is NOT marked as `managed`, unset the resource ID
// of the newly-created `cloudstack_firewall` resource, as this should only happen IF
// the resource does not actually exist during Refresh, and should be removed from Terraform state.
} else if !managed {
d.SetId("")
} The logic here is correct - during a normal resource read, if However, since // ...
// The SetId method must be called with a non-empty value for the managed
// resource instance to be properly saved into the Terraform state and
// avoid a "inconsistent result after apply" error.
// ...
Create CreateFunc This is why your fix appears to work: if you change Since --- a/cloudstack/resource_cloudstack_firewall.go
+++ b/cloudstack/resource_cloudstack_firewall.go
@@ -95,6 +95,12 @@ func resourceCloudStackFirewall() *schema.Resource {
},
},
+ "project": {
+ Type: schema.TypeString,
+ Optional: true,
+ ForceNew: true,
+ },
+
"parallelism": {
Type: schema.TypeInt,
Optional: true,
@@ -256,6 +262,12 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
p.SetIpaddressid(d.Id())
p.SetListall(true)
+ // Use WithProject(d.Get("project").(string)) to set the `projectid` parameter on the API call
+ // if we've passed a `project` parameter to the resource
+ if err := cloudstack.WithProject(d.Get("project").(string))(cs, p); err != nil {
+ return err
+ }
+
l, err := cs.Firewall.ListFirewallRules(p)
if err != nil {
return err I'm not in a position to test this fix myself right now as I'm not currently using firewall rules in my deployment, but I'm fairly confident it should work. Side note: we have to supply the |
Hi,
this is my first try to open an issue here, please bear with me, if this is not the correct way. I will thankfully accept any hints to optimize it in the future. I will try to orientate to the issue-template from the cloudstack-project.
I may repeat this with different tf-versions / os'es. Let me know, if you need more information.
PROVIDER INFORMATION
TERRAFORM INFORMATION
HYPERVISOR INFORMATION
CLOUDSTACK VERSION
CONFIGURATION
OS / ENVIRONMENT
SUMMARY
when trying to deploy a firewall-rule to a simple isolated guest-network via cloudstack-provider.
DETAILS
STEPS TO REPRODUCE
EXPECTED RESULTS
ACTUAL RESULTS
Rule is created successfully
Rule is not included in terraform state (Rerun will try to create a new rule)
Full Output:
The text was updated successfully, but these errors were encountered: