Skip to content
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

adding security policy field to instance #8878

Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d0f4dc2
adding security policy field to networkInterfaceAccessConfig
felipegc Sep 6, 2023
9487881
adding security policy to networkInterface instead of networkInterfac…
felipegc Sep 6, 2023
3087f96
finishing solution 1 and adding integration tests and doc
felipegc Sep 12, 2023
e2ecc70
cleanups for solution 1
felipegc Sep 12, 2023
130efc5
wrapping update security policy for beta
felipegc Sep 12, 2023
37ea464
Merge remote-tracking branch 'upstream/main' into add-region-security…
felipegc Sep 12, 2023
0d57345
replacing the networks in tests
felipegc Sep 12, 2023
4b109d8
Merge remote-tracking branch 'upstream/main' into add-region-security…
felipegc Sep 14, 2023
9f93eb7
fixing code review by implementing the solution two
felipegc Sep 14, 2023
dd8f9df
replacing networks in tests
felipegc Sep 14, 2023
24da3c2
fixing nic read for instance_template resource
felipegc Sep 15, 2023
c9347f7
adding checking for access config security policy while flattening to…
felipegc Sep 15, 2023
22bef82
fixing error while creating instance with empty security_policy
felipegc Sep 15, 2023
ade8d0e
Merge remote-tracking branch 'upstream/main' into add-region-security…
felipegc Sep 18, 2023
a726dc6
changing region for tests which use network_edge_security_service
felipegc Sep 18, 2023
240d3c5
Merge remote-tracking branch 'upstream/main' into add-region-security…
felipegc Sep 18, 2023
fa96e75
comment all failing tests but one to test if it runs alone
felipegc Sep 18, 2023
9b7b66c
making the tests running serially
felipegc Sep 18, 2023
42920fb
fixing the tests to be called only by the serial one
felipegc Sep 19, 2023
4c915c5
fixing code review comments
felipegc Sep 20, 2023
2b5e5be
Merge remote-tracking branch 'upstream/main' into add-region-security…
felipegc Sep 21, 2023
a90ca86
fixing missing compute_image data from merge
felipegc Sep 21, 2023
63e8ffa
fixing code review
felipegc Sep 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 All @@ -387,6 +406,11 @@ func expandAccessConfigs(configs []interface{}) []*compute.AccessConfig {
acs[i].SetPublicPtr = true
acs[i].PublicPtrDomainName = ptr.(string)
}
<% unless version == 'ga' -%>
if sp, ok := data["security_policy"]; ok && sp != "" {
acs[i].SecurityPolicy = sp.(string)
}
<% end -%>
felipegc marked this conversation as resolved.
Show resolved Hide resolved
}
}
return acs
Expand Down Expand Up @@ -416,6 +440,11 @@ func expandIpv6AccessConfigs(configs []interface{}) []*compute.AccessConfig {
iacs[i].Name = name.(string)
}
iacs[i].Type = "DIRECT_IPV6" // Currently only type supported
<% unless version == 'ga' -%>
if sp, ok := data["security_policy"]; ok && sp != "" {
iacs[i].SecurityPolicy = sp.(string)
felipegc marked this conversation as resolved.
Show resolved Hide resolved
}
<% end -%>
}
}
return iacs
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") {
zli82016 marked this conversation as resolved.
Show resolved Hide resolved
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)
felipegc marked this conversation as resolved.
Show resolved Hide resolved
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