Skip to content

Commit

Permalink
adding security policy field to instance (GoogleCloudPlatform#8878)
Browse files Browse the repository at this point in the history
* adding security policy field to networkInterfaceAccessConfig

* adding security policy to networkInterface instead of networkInterfaceAccessConfig

* finishing solution 1 and adding integration tests and doc

* cleanups for solution 1

* wrapping update security policy for beta

* replacing the networks in tests

* fixing code review by implementing the solution two

* replacing networks in tests

* fixing nic read for instance_template resource

* adding checking for access config security policy while flattening to prevent the instance template to break

* fixing error while creating instance with empty security_policy

* changing region for tests which use network_edge_security_service

* comment all failing tests but one to test if it runs alone

* making the tests running serially

* fixing the tests to be called only by the serial one

* fixing code review comments

* fixing missing compute_image data from merge

* fixing code review
  • Loading branch information
felipegc authored and nevzheng committed Sep 22, 2023
1 parent e567e95 commit c276820
Show file tree
Hide file tree
Showing 5 changed files with 1,346 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,11 @@ func flattenAccessConfigs(accessConfigs []*compute.AccessConfig) ([]map[string]i
if natIP == "" {
natIP = ac.NatIP
}
<% unless version == 'ga' -%>
if ac.SecurityPolicy != "" {
flattened[i]["security_policy"] = ac.SecurityPolicy
}
<% end -%>
}
return flattened, natIP
}
Expand All @@ -319,6 +324,11 @@ func flattenIpv6AccessConfigs(ipv6AccessConfigs []*compute.AccessConfig) []map[s
flattened[i]["external_ipv6"] = ac.ExternalIpv6
flattened[i]["external_ipv6_prefix_length"] = strconv.FormatInt(ac.ExternalIpv6PrefixLength, 10)
flattened[i]["name"] = ac.Name
<% unless version == 'ga' -%>
if ac.SecurityPolicy != "" {
flattened[i]["security_policy"] = ac.SecurityPolicy
}
<% end -%>
}
return flattened
}
Expand Down Expand Up @@ -370,6 +380,15 @@ func flattenNetworkInterfaces(d *schema.ResourceData, config *transport_tpg.Conf
flattened[i]["network_attachment"] = networkAttachment
}
<% end -%>

<% unless version == 'ga' -%>
// the security_policy for a network_interface is found in one of its accessConfigs.
if len(iface.AccessConfigs) > 0 && iface.AccessConfigs[0].SecurityPolicy != "" {
flattened[i]["security_policy"] = iface.AccessConfigs[0].SecurityPolicy
} else if len(iface.Ipv6AccessConfigs) > 0 && iface.Ipv6AccessConfigs[0].SecurityPolicy != "" {
flattened[i]["security_policy"] = iface.Ipv6AccessConfigs[0].SecurityPolicy
}
<% end -%>
}
return flattened, region, internalIP, externalIP, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import (
transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport"
"github.com/hashicorp/errwrap"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
<% unless version == 'ga' -%>
"github.com/hashicorp/terraform-provider-google/google/tpgresource"
<% end -%>

<% if version == "ga" -%>
"google.golang.org/api/compute/v1"
<% else -%>
Expand Down Expand Up @@ -85,3 +89,78 @@ func computeInstanceCreateUpdateWhileStoppedCall(d *schema.ResourceData, config
return nil
}
}

<% unless version == 'ga' -%>
func computeInstanceAddSecurityPolicy(d *schema.ResourceData, config *transport_tpg.Config, securityPolicyWithNics map[string][]string, project, zone, userAgent, instanceName string) error {
for sp, nics := range securityPolicyWithNics {
req := &compute.InstancesSetSecurityPolicyRequest{
NetworkInterfaces: nics,
SecurityPolicy: sp,
}
op, err := config.NewComputeClient(userAgent).Instances.SetSecurityPolicy(project, zone, instanceName, req).Do()
if err != nil {
return fmt.Errorf("Error adding security policy: %s", err)
}
opErr := ComputeOperationWaitTime(config, op, project, "security_policy to add", userAgent, d.Timeout(schema.TimeoutUpdate))
if opErr != nil {
return opErr
}
}

return nil
}

func computeInstanceMapSecurityPoliciesCreate(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (map[string][]string, error) {
securityPolicies := make(map[string][]string)
configs := d.Get("network_interface").([]interface{})
for i, raw := range configs {
data := raw.(map[string]interface{})
secPolicy := data["security_policy"].(string)
err := validateSecurityPolicy(data)
if err != nil {
return securityPolicies, err
}

if secPolicy != "" {
// Network interfaces use the nicN naming format and is only know after the instance is created.
nicName := fmt.Sprintf("nic%d", i)
securityPolicies[secPolicy] = append(securityPolicies[secPolicy], nicName)
}
}

return securityPolicies, nil
}

func computeInstanceMapSecurityPoliciesUpdate(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (map[string][]string, error) {
securityPolicies := make(map[string][]string)
configs := d.Get("network_interface").([]interface{})
for i, raw := range configs {
data := raw.(map[string]interface{})
secPolicy := data["security_policy"].(string)
err := validateSecurityPolicy(data)
if err != nil {
return securityPolicies, err
}

// Network interfaces use the nicN naming format and is only know after the instance is created.
nicName := fmt.Sprintf("nic%d", i)
// To cleanup the security policy from the interface we should send something like this on the api: {"":[nic0, nic1]}
securityPolicies[secPolicy] = append(securityPolicies[secPolicy], nicName)
}

return securityPolicies, nil
}

func validateSecurityPolicy(rawNetworkInterface map[string]interface{}) error {
acessConfigs := expandAccessConfigs(rawNetworkInterface["access_config"].([]interface{}))
ipv6AccessConfigs := expandIpv6AccessConfigs(rawNetworkInterface["ipv6_access_config"].([]interface{}))
secPolicy := rawNetworkInterface["security_policy"].(string)

if secPolicy != "" && len(acessConfigs) == 0 && len(ipv6AccessConfigs) == 0 {
return fmt.Errorf("Error setting security policy to the instance since at least one access config must exist")
}

return nil
}

<% end -%>
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,13 @@ func ResourceComputeInstance() *schema.Resource {
Optional: true,
Description: `The DNS domain name for the public PTR record.`,
},
<% unless version == 'ga' -%>
"security_policy": {
Type: schema.TypeString,
Computed: true,
Description: `A full or partial URL to a security policy to add to this instance. If this field is set to an empty string it will remove the associated security policy.`,
},
<% end -%>
},
},
},
Expand Down Expand Up @@ -444,6 +451,13 @@ func ResourceComputeInstance() *schema.Resource {
ForceNew: true,
Description: `The name of this access configuration. In ipv6AccessConfigs, the recommended name is External IPv6.`,
},
<% unless version == 'ga' -%>
"security_policy": {
Type: schema.TypeString,
Computed: true,
Description: `A full or partial URL to a security policy to add to this instance. If this field is set to an empty string it will remove the associated security policy.`,
},
<% end -%>
},
},
},
Expand All @@ -469,6 +483,14 @@ func ResourceComputeInstance() *schema.Resource {
ForceNew: true,
Description: `The networking queue count that's specified by users for the network interface. Both Rx and Tx queues will be set to this number. It will be empty if not specified.`,
},

<% unless version == 'ga' -%>
"security_policy": {
Type: schema.TypeString,
Optional: true,
Description: `A full or partial URL to a security policy to add to this instance. If this field is set to an empty string it will remove the associated security policy.`,
},
<% end -%>
},
},
},
Expand Down Expand Up @@ -1300,6 +1322,13 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
return err
}

<% unless version == 'ga' -%>
securityPolicies, err := computeInstanceMapSecurityPoliciesCreate(d, config)
if err != nil {
return err
}
<% end -%>

log.Printf("[INFO] Requesting instance creation")
op, err := config.NewComputeClient(userAgent).Instances.Insert(project, zone.Name, instance).Do()
if err != nil {
Expand All @@ -1317,6 +1346,13 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
return waitErr
}

<% unless version == 'ga' -%>
err = computeInstanceAddSecurityPolicy(d, config, securityPolicies, project, z, userAgent, instance.Name)
if err != nil {
return fmt.Errorf("Error creating instance while setting the security policies: %s", err)
}
<% end -%>

err = waitUntilInstanceHasDesiredStatus(config, d)
if err != nil {
return fmt.Errorf("Error waiting for status: %s", err)
Expand Down Expand Up @@ -1759,6 +1795,30 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
return fmt.Errorf("Instance had unexpected number of network interfaces: %d", len(instance.NetworkInterfaces))
}

<% unless version == 'ga' -%>
updateSecurityPolicy := false
for i := 0; i < len(instance.NetworkInterfaces); i++ {
prefix := fmt.Sprintf("network_interface.%d", i)
// check if sec policy has been changed
// check if access config has been changed because it may be deleted and needs to be re-created.
if d.HasChange(prefix+".security_policy") || d.HasChange(prefix+".access_config") || d.HasChange(prefix+".ipv6_access_config") {
if instance.Status != "RUNNING" {
return fmt.Errorf("Error to update security policy because the current instance status must be \"RUNNING\". The security policy or some access config may have changed which requires the security policy to be re-applied")
}
updateSecurityPolicy = true
}
}

securityPolicies := make(map[string][]string)
if updateSecurityPolicy {
// map the security policies to call SetSecurityPolicy because the next section of the code removes and re-creates the access_config which ends up removing the security_policy.
securityPolicies, err = computeInstanceMapSecurityPoliciesUpdate(d, config)
if err != nil {
return err
}
}
<% end -%>

var updatesToNIWhileStopped []func(inst *compute.Instance) error
for i := 0; i < len(networkInterfaces); i++ {
prefix := fmt.Sprintf("network_interface.%d", i)
Expand Down Expand Up @@ -2300,6 +2360,14 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
}
}

<% unless version == 'ga' -%>
// The access config must be updated only if the machine is still RUNNING and after each access_config for each interface has been re-created.
err = computeInstanceAddSecurityPolicy(d, config, securityPolicies, project, zone, userAgent, instance.Name)
if err != nil {
return fmt.Errorf("Error updating instance while setting the security policies: %s", err)
}
<% end -%>

// We made it, disable partial mode
d.Partial(false)

Expand Down
Loading

0 comments on commit c276820

Please sign in to comment.