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.

Sign up for GitHub

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

feat(CLOUDDEV-420): added disable_port_security option to instance re… #38

Merged
merged 1 commit into from
Jan 18, 2024
Merged

feat(CLOUDDEV-420): added disable_port_security option to instance re… #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions docs/resources/instance.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,11 @@ resource "edgecenter_instance" "instance" {
}

interface {
type = "subnet"
network_id = edgecenter_network.network.id
subnet_id = edgecenter_subnet.subnet.id
security_groups = ["d75db0b2-58f1-4a11-88c6-a932bb897310"]
type = "subnet"
network_id = edgecenter_network.network.id
subnet_id = edgecenter_subnet.subnet.id
security_groups = ["d75db0b2-58f1-4a11-88c6-a932bb897310"]
port_security_disabled = true
}

metadata_map = {
Expand Down Expand Up @@ -199,6 +200,7 @@ Optional:
- `network_id` (String) Required if type is 'subnet' or 'any_subnet'.
- `order` (Number) Order of attaching interface
- `port_id` (String) required if type is 'reserved_fixed_ip'
- `port_security_disabled` (Boolean)
- `security_groups` (List of String) list of security group IDs
- `subnet_id` (String) Required if type is 'subnet'.
- `type` (String) Available value is 'subnet', 'any_subnet', 'external', 'reserved_fixed_ip'
Expand Down
56 changes: 44 additions & 12 deletions edgecenter/resource_edgecenter_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
InstanceDeleting int = 1200
InstanceCreatingTimeout int = 1200
InstancePoint = "instances"
PortsPoint = "ports"

InstanceVMStateActive = "active"
InstanceVMStateStopped = "stopped"
Expand Down Expand Up @@ -223,6 +224,11 @@ func resourceInstance() *schema.Resource {
Computed: true,
Optional: true,
},
"port_security_disabled": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
},
},
},
},
Expand Down Expand Up @@ -394,12 +400,12 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, m inter
config := m.(*Config)
provider := config.Provider

clientV1, err := CreateClient(provider, d, InstancePoint, VersionPointV1)
instancesClientV1, err := CreateClient(provider, d, InstancePoint, VersionPointV1)
if err != nil {
return diag.FromErr(err)
}

clientV2, err := CreateClient(provider, d, InstancePoint, VersionPointV2)
instancesClientV2, err := CreateClient(provider, d, InstancePoint, VersionPointV2)
if err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -449,11 +455,11 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, m inter

ifs := d.Get("interface").([]interface{})
if len(ifs) > 0 {
interfacesList, err := extractInstanceInterfaceToListCreate(ifs)
ifaceCreateOptsList, err := extractInstanceInterfaceToListCreate(ifs)
if err != nil {
return diag.FromErr(err)
}
createOpts.Interfaces = interfacesList
createOpts.Interfaces = ifaceCreateOptsList
}

if metadata, ok := d.GetOk("metadata"); ok {
Expand All @@ -479,15 +485,15 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, m inter
}

log.Printf("[DEBUG] Instance create options: %+v", createOpts)
results, err := instances.Create(clientV2, createOpts).Extract()
results, err := instances.Create(instancesClientV2, createOpts).Extract()
if err != nil {
return diag.FromErr(err)
}

taskID := results.Tasks[0]
log.Printf("[DEBUG] Task id (%s)", taskID)
InstanceID, err := tasks.WaitTaskAndReturnResult(clientV1, taskID, true, InstanceCreatingTimeout, func(task tasks.TaskID) (interface{}, error) {
taskInfo, err := tasks.Get(clientV1, string(task)).Extract()
InstanceID, err := tasks.WaitTaskAndReturnResult(instancesClientV1, taskID, true, InstanceCreatingTimeout, func(task tasks.TaskID) (interface{}, error) {
taskInfo, err := tasks.Get(instancesClientV1, string(task)).Extract()
if err != nil {
return nil, fmt.Errorf("cannot get task with ID: %s. Error: %w", task, err)
}
Expand All @@ -498,11 +504,32 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, m inter
return Instance, nil
},
)
log.Printf("[DEBUG] Instance id (%s)", InstanceID)
if err != nil {
return diag.FromErr(err)
}

instanceID := InstanceID.(string)

// Code below adjusts all interfaces PortSecurityDisabled opt
interfacesListAPI, err := instances.ListInterfacesAll(instancesClientV1, instanceID)
if err != nil {
return diag.FromErr(fmt.Errorf("error from getting instance interfaces: %w", err))
}

portsClientV1, err := CreateClient(provider, d, PortsPoint, VersionPointV1)
if err != nil {
return diag.FromErr(fmt.Errorf("error from creating ports client: %w", err))
}
for _, iface := range ifs {
ifaceMap := iface.(map[string]interface{})
err = adjustPortSecurityDisabledOpt(portsClientV1, interfacesListAPI, ifaceMap)
if err != nil {
return diag.FromErr(fmt.Errorf("error from port securtity disable option configuring. Interface: %#v, error: %w", ifaceMap, err))
}
}

log.Printf("[DEBUG] Instance id (%s)", InstanceID)

d.SetId(InstanceID.(string))
resourceInstanceRead(ctx, d, m)

Expand Down Expand Up @@ -620,6 +647,7 @@ func resourceInstanceRead(_ context.Context, d *schema.ResourceData, m interface
i["network_id"] = iFace.NetworkID
i["subnet_id"] = subnetID
i["port_id"] = portID
i["port_security_disabled"] = !iFace.PortSecurityEnabled
if interfaceOpts.FloatingIP != nil {
i["fip_source"] = interfaceOpts.FloatingIP.Source.String()
i["existing_fip_id"] = interfaceOpts.FloatingIP.ExistingFloatingID
Expand Down Expand Up @@ -798,6 +826,10 @@ func resourceInstanceUpdate(ctx context.Context, d *schema.ResourceData, m inter
}

if d.HasChange("interface") {
portsClientV1, err := CreateClient(provider, d, PortsPoint, VersionPointV1)
if err != nil {
return diag.FromErr(err)
}
iOldRaw, iNewRaw := d.GetChange("interface")
ifsOldSlice, ifsNewSlice := iOldRaw.([]interface{}), iNewRaw.([]interface{})
sort.Sort(instanceInterfaces(ifsOldSlice))
Expand Down Expand Up @@ -833,7 +865,7 @@ func resourceInstanceUpdate(ctx context.Context, d *schema.ResourceData, m inter
if err := detachInterfaceFromInstance(client, instanceID, iOld); err != nil {
return diag.FromErr(err)
}
if err := attachInterfaceToInstance(client, instanceID, iNew); err != nil {
if err := attachInterfaceToInstance(client, portsClientV1, instanceID, iNew); err != nil {
return diag.FromErr(err)
}
}
Expand Down Expand Up @@ -869,15 +901,15 @@ func resourceInstanceUpdate(ctx context.Context, d *schema.ResourceData, m inter
if err := detachInterfaceFromInstance(client, instanceID, iOld); err != nil {
return diag.FromErr(err)
}
if err := attachInterfaceToInstance(client, instanceID, iNew); err != nil {
if err := attachInterfaceToInstance(client, portsClientV1, instanceID, iNew); err != nil {
return diag.FromErr(err)
}
}
}

for _, item := range ifsNewSlice[len(ifsOldSlice):] {
iNew := item.(map[string]interface{})
if err := attachInterfaceToInstance(client, instanceID, iNew); err != nil {
if err := attachInterfaceToInstance(client, portsClientV1, instanceID, iNew); err != nil {
return diag.FromErr(err)
}
}
Expand Down Expand Up @@ -912,7 +944,7 @@ func resourceInstanceUpdate(ctx context.Context, d *schema.ResourceData, m inter
if err := detachInterfaceFromInstance(client, instanceID, iOld); err != nil {
return diag.FromErr(err)
}
if err := attachInterfaceToInstance(client, instanceID, iNew); err != nil {
if err := attachInterfaceToInstance(client, portsClientV1, instanceID, iNew); err != nil {
return diag.FromErr(err)
}
}
Expand Down
64 changes: 61 additions & 3 deletions edgecenter/utils_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
edgecloud "github.com/Edge-Center/edgecentercloud-go"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/instance/v1/instances"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/instance/v1/types"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/port/v1/ports"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/securitygroup/v1/securitygroups"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/servergroup/v1/servergroups"
"github.com/Edge-Center/edgecentercloud-go/edgecenter/task/v1/tasks"
Expand All @@ -26,6 +27,13 @@ var instanceDecoderConfig = &mapstructure.DecoderConfig{

type instanceInterfaces []interface{}

type InstancePortSecurityOpts struct {
PortID string
PortSecurityDisabled bool
SubnetID string
IPAddress string
}

func (s instanceInterfaces) Len() int {
return len(s)
}
Expand Down Expand Up @@ -341,13 +349,13 @@ func detachInterfaceFromInstance(client *edgecloud.ServiceClient, instanceID str
}

// attachInterfaceToInstance attach interface to instance.
func attachInterfaceToInstance(instanceClient *edgecloud.ServiceClient, instanceID string, iface map[string]interface{}) error {
func attachInterfaceToInstance(instanceClient, portsClient *edgecloud.ServiceClient, instanceID string, iface map[string]interface{}) error {
iType := types.InterfaceType(iface["type"].(string))
opts := instances.InterfaceInstanceCreateOpts{
InterfaceOpts: instances.InterfaceOpts{Type: iType},
}

switch iType { //nolint: exhaustive
switch iType { // nolint: exhaustive
case types.SubnetInterfaceType:
opts.SubnetID = iface["subnet_id"].(string)
case types.AnySubnetInterfaceType:
Expand Down Expand Up @@ -382,6 +390,56 @@ func attachInterfaceToInstance(instanceClient *edgecloud.ServiceClient, instance
return err
}

interfacesListAPI, err := instances.ListInterfacesAll(instanceClient, instanceID)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Когда функция большая и состоит из нескольких этапов, можно выделить явные шаги. Например, выше у тебя происходил attach интерфейса, а ниже приведение поля disable_port_security в соответствии с его значением в opts.

Я обычно подписываю такие шаги, чтобы легче было читать код.

Предлагаю тебе еще раз взглянуть на твой код и посмотреть, будет ли здесь актуально тоже подписать, что это за большой шаг внутри функции

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Долго читал функцию, особенно меня смутило, почему у тебя в цикле проверяется opts, если opts -- это объект, сформированный только для запроса выше в функции.

Это наглядный пример, почему стоит когда это возможно следить за тем, чтобы функция выполняла ровно одну задачу. Иначе нужно внимательно следить, чтобы переменные, предназначеные для чего-то одного, не использовались там, где они не предназначена.

if err != nil {
return fmt.Errorf("error from getting instance interfaces: %w", err)
}

if err = adjustPortSecurityDisabledOpt(portsClient, interfacesListAPI, iface); err != nil {
return fmt.Errorf("cannot adjust port_security_disabled opt: %+v. Error: %w", iface, err)
}

return nil
}

// adjustPortSecurityDisabledOpt aligns the state of the interface (port_security_disabled) with what is specified in the
// iface["port_security_disabled"].
func adjustPortSecurityDisabledOpt(portsClient *edgecloud.ServiceClient, interfacesListAPI []instances.Interface, iface map[string]interface{}) error {
portSecurityDisabled := iface["port_security_disabled"].(bool)
IPAddress := iface["ip_address"].(string)
subnetID := iface["subnet_id"].(string)
ifacePortID := iface["port_id"].(string)

LOOP:
for _, iFace := range interfacesListAPI {
if len(iFace.IPAssignments) == 0 {
continue
}

requestedIfacePortID := iFace.PortID
for _, assignment := range iFace.IPAssignments {
requestedIfaceSubnetID := assignment.SubnetID
requestedIfaceIPAddress := assignment.IPAddress.String()

if subnetID == requestedIfaceSubnetID || IPAddress == requestedIfaceIPAddress || ifacePortID == requestedIfacePortID {
if !iFace.PortSecurityEnabled != portSecurityDisabled {
switch portSecurityDisabled {
case true:
if _, err := ports.DisablePortSecurity(portsClient, requestedIfacePortID).Extract(); err != nil {
return err
}
case false:
if _, err := ports.EnablePortSecurity(portsClient, requestedIfacePortID).Extract(); err != nil {
return err
}
}
}

break LOOP
}
}
}

return nil
}

Expand Down Expand Up @@ -511,7 +569,7 @@ func getSecurityGroupsIDs(sgsRaw []interface{}) []edgecloud.ItemID {
}

// getSecurityGroupsDifference finds the difference between two slices of edgecloud.ItemID.
func getSecurityGroupsDifference(sl1, sl2 []edgecloud.ItemID) (diff []edgecloud.ItemID) { //nolint: nonamedreturns
func getSecurityGroupsDifference(sl1, sl2 []edgecloud.ItemID) (diff []edgecloud.ItemID) { // nolint: nonamedreturns
set := make(map[string]bool)
for _, item := range sl1 {
set[item.ID] = true
Expand Down
9 changes: 5 additions & 4 deletions examples/resources/edgecenter_instance/resource.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ resource "edgecenter_instance" "instance" {
}

interface {
type = "subnet"
network_id = edgecenter_network.network.id
subnet_id = edgecenter_subnet.subnet.id
security_groups = ["d75db0b2-58f1-4a11-88c6-a932bb897310"]
type = "subnet"
network_id = edgecenter_network.network.id
subnet_id = edgecenter_subnet.subnet.id
security_groups = ["d75db0b2-58f1-4a11-88c6-a932bb897310"]
port_security_disabled = true
}

metadata_map = {
Expand Down