diff --git a/internal/server/network/acl/acl_ovn.go b/internal/server/network/acl/acl_ovn.go index ced8a0bbaf2..508da75a8a9 100644 --- a/internal/server/network/acl/acl_ovn.go +++ b/internal/server/network/acl/acl_ovn.go @@ -403,7 +403,7 @@ func ovnApplyToPortGroup(l logger.Logger, client *ovn.NB, aclInfo *api.NetworkAC } // Clear all existing ACL rules from port group then add the new rules to the port group. - err = client.PortGroupSetACLRules(portGroupName, nil, portGroupRules...) + err = client.UpdatePortGroupACLRules(context.TODO(), portGroupName, nil, portGroupRules...) if err != nil { return fmt.Errorf("Failed applying ACL %q rules to port group %q: %w", aclInfo.Name, portGroupName, err) } @@ -419,7 +419,7 @@ func ovnApplyToPortGroup(l logger.Logger, client *ovn.NB, aclInfo *api.NetworkAC fmt.Sprintf("@%s", ruleSubjectExternal): fmt.Sprintf(`"%s"`, OVNIntSwitchRouterPortName(aclNet.ID)), } - err = client.PortGroupSetACLRules(netPortGroupName, matchReplace, networkRules...) + err = client.UpdatePortGroupACLRules(context.TODO(), netPortGroupName, matchReplace, networkRules...) if err != nil { return fmt.Errorf("Failed applying ACL %q rules to port group %q for network %q: %w", aclInfo.Name, netPortGroupName, aclNet.Name, err) } @@ -746,7 +746,7 @@ func OVNApplyNetworkBaselineRules(client *ovn.NB, switchName ovn.OVNSwitch, rout ) } - err := client.LogicalSwitchSetACLRules(switchName, rules...) + err := client.UpdateLogicalSwitchACLRules(context.TODO(), switchName, rules...) if err != nil { return fmt.Errorf("Failed applying baseline ACL rules to logical switch %q: %w", switchName, err) } @@ -1024,7 +1024,7 @@ func OVNApplyInstanceNICDefaultRules(client *ovn.NB, switchPortGroup ovn.OVNPort }, } - err := client.PortGroupPortSetACLRules(switchPortGroup, nicPortName, rules...) + err := client.UpdatePortGroupPortACLRules(context.TODO(), switchPortGroup, nicPortName, rules...) if err != nil { return fmt.Errorf("Failed applying instance NIC default ACL rules for port %q: %w", nicPortName, err) } diff --git a/internal/server/network/driver_ovn.go b/internal/server/network/driver_ovn.go index 374c18717d9..220fb0fc052 100644 --- a/internal/server/network/driver_ovn.go +++ b/internal/server/network/driver_ovn.go @@ -3143,7 +3143,7 @@ func (n *ovn) Update(newNetwork api.NetworkPut, targetNode string, clientType re // If there are no ACLs being applied to the NIC (either from network or NIC) then // we should remove the default rule from the NIC. if len(newACLs) <= 0 && len(nicACLs) <= 0 { - err = n.state.OVNNB.PortGroupPortClearACLRules(acl.OVNIntSwitchPortGroupName(n.ID()), instancePortName) + err = n.state.OVNNB.ClearPortGroupPortACLRules(context.TODO(), acl.OVNIntSwitchPortGroupName(n.ID()), instancePortName) if err != nil { return fmt.Errorf("Failed clearing OVN default ACL rules for instance NIC: %w", err) } @@ -3425,7 +3425,9 @@ func (n *ovn) InstanceDevicePortAdd(instanceUUID string, deviceName string, devi return fmt.Errorf("Failed adding DNS record: %w", err) } - revert.Add(func() { _ = n.state.OVNNB.LogicalSwitchPortDeleteDNS(n.getIntSwitchName(), dnsUUID, true) }) + revert.Add(func() { + _ = n.state.OVNNB.DeleteLogicalSwitchPortDNS(context.TODO(), n.getIntSwitchName(), dnsUUID, true) + }) // If NIC has static IPv4 address then create a DHCPv4 reservation. if deviceConfig["ipv4.address"] != "" { @@ -3704,7 +3706,9 @@ func (n *ovn) InstanceDevicePortStart(opts *OVNInstanceNICSetupOpts, securityACL return "", nil, fmt.Errorf("Failed setting DNS for %q: %w", dnsName, err) } - revert.Add(func() { _ = n.state.OVNNB.LogicalSwitchPortDeleteDNS(n.getIntSwitchName(), dnsUUID, false) }) + revert.Add(func() { + _ = n.state.OVNNB.DeleteLogicalSwitchPortDNS(context.TODO(), n.getIntSwitchName(), dnsUUID, false) + }) // If NIC has static IPv4 address then ensure a DHCPv4 reservation exists. // Do this at start time as well as add time in case an instance was copied (causing a duplicate address @@ -4000,7 +4004,7 @@ func (n *ovn) InstanceDevicePortStart(opts *OVNInstanceNICSetupOpts, securityACL n.logger.Debug("Set NIC default rule", logger.Ctx{"port": instancePortName, "ingressAction": ingressAction, "ingressLogged": ingressLogged, "egressAction": egressAction, "egressLogged": egressLogged}) } else { - err = n.state.OVNNB.PortGroupPortClearACLRules(acl.OVNIntSwitchPortGroupName(n.ID()), instancePortName) + err = n.state.OVNNB.ClearPortGroupPortACLRules(context.TODO(), acl.OVNIntSwitchPortGroupName(n.ID()), instancePortName) if err != nil { return "", nil, fmt.Errorf("Failed clearing OVN default ACL rules for instance NIC: %w", err) } @@ -4099,7 +4103,7 @@ func (n *ovn) InstanceDevicePortStop(ovsExternalOVNPort networkOVN.OVNSwitchPort } // Cleanup logical switch port and associated config. - err = n.state.OVNNB.LogicalSwitchPortCleanup(instancePortName, n.getIntSwitchName(), acl.OVNIntSwitchPortGroupName(n.ID()), dnsUUID) + err = n.state.OVNNB.CleanupLogicalSwitchPort(context.TODO(), instancePortName, n.getIntSwitchName(), acl.OVNIntSwitchPortGroupName(n.ID()), dnsUUID) if err != nil { return err } @@ -4228,7 +4232,7 @@ func (n *ovn) InstanceDevicePortRemove(instanceUUID string, deviceName string, d } } - err = n.state.OVNNB.LogicalSwitchPortDeleteDNS(n.getIntSwitchName(), dnsUUID, true) + err = n.state.OVNNB.DeleteLogicalSwitchPortDNS(context.TODO(), n.getIntSwitchName(), dnsUUID, true) if err != nil { return fmt.Errorf("Failed deleting DNS record: %w", err) } diff --git a/internal/server/network/ovn/ovn_nb.go b/internal/server/network/ovn/ovn_nb.go index 13a70281d1e..35e52674893 100644 --- a/internal/server/network/ovn/ovn_nb.go +++ b/internal/server/network/ovn/ovn_nb.go @@ -6,7 +6,6 @@ import ( "crypto/x509" "encoding/pem" "fmt" - "os" "reflect" "runtime" "strings" @@ -17,21 +16,13 @@ import ( ovsdbClient "github.com/ovn-org/libovsdb/client" ovsdbModel "github.com/ovn-org/libovsdb/model" - "github.com/lxc/incus/v6/internal/linux" ovnNB "github.com/lxc/incus/v6/internal/server/network/ovn/schema/ovn-nb" - "github.com/lxc/incus/v6/shared/subprocess" ) // NB client. type NB struct { client ovsdbClient.Client cookie ovsdbClient.MonitorCookie - - // For nbctl command calls. - dbAddr string - sslCACert string - sslClientCert string - sslClientKey string } var nb *NB @@ -43,9 +34,7 @@ func NewNB(dbAddr string, sslCACert string, sslClientCert string, sslClientKey s } // Create the NB struct. - client := &NB{ - dbAddr: dbAddr, - } + client := &NB{} // Prepare the OVSDB client. dbSchema, err := ovnNB.FullDatabaseModel() @@ -140,11 +129,6 @@ func NewNB(dbAddr string, sslCACert string, sslClientCert string, sslClientKey s // Add the TLS config to the client. options = append(options, ovsdbClient.WithTLSConfig(tlsConfig)) - - // Fill the fields need for the CLI calls. - client.sslCACert = sslCACert - client.sslClientCert = sslClientCert - client.sslClientKey = sslClientKey } // Connect to OVSDB. @@ -230,49 +214,3 @@ func (o *NB) get(ctx context.Context, m ovsdbModel.Model) error { reflect.ValueOf(m).Elem().Set(rVal.Index(0)) return nil } - -// nbctl executes ovn-nbctl with arguments to connect to wrapper's northbound database. -func (o *NB) nbctl(extraArgs ...string) (string, error) { - // Figure out args. - args := []string{"--timeout=10", "--db", o.dbAddr} - - // Handle SSL args. - files := []*os.File{} - if strings.Contains(o.dbAddr, "ssl:") { - // Handle client certificate. - clientCertFile, err := linux.CreateMemfd([]byte(o.sslClientCert)) - if err != nil { - return "", err - } - - defer clientCertFile.Close() - files = append(files, clientCertFile) - - // Handle client key. - clientKeyFile, err := linux.CreateMemfd([]byte(o.sslClientKey)) - if err != nil { - return "", err - } - - defer clientKeyFile.Close() - files = append(files, clientKeyFile) - - // Handle CA certificate. - caCertFile, err := linux.CreateMemfd([]byte(o.sslCACert)) - if err != nil { - return "", err - } - - defer caCertFile.Close() - files = append(files, caCertFile) - - args = append(args, - "-c", "/proc/self/fd/3", - "-p", "/proc/self/fd/4", - "-C", "/proc/self/fd/5", - ) - } - - args = append(args, extraArgs...) - return subprocess.RunCommandInheritFds(context.Background(), files, "ovn-nbctl", args...) -} diff --git a/internal/server/network/ovn/ovn_nb_actions.go b/internal/server/network/ovn/ovn_nb_actions.go index 42688449e0e..8fecc7eb67b 100644 --- a/internal/server/network/ovn/ovn_nb_actions.go +++ b/internal/server/network/ovn/ovn_nb_actions.go @@ -5,7 +5,6 @@ import ( "fmt" "net" "slices" - "strconv" "strings" "time" @@ -1425,19 +1424,49 @@ func (o *NB) DeleteLogicalSwitchDHCPOption(ctx context.Context, switchName OVNSw return nil } -// LogicalSwitchSetACLRules applies a set of rules to the specified logical switch. Any existing rules are removed. -func (o *NB) LogicalSwitchSetACLRules(switchName OVNSwitch, aclRules ...OVNACLRule) error { +// UpdateLogicalSwitchACLRules applies a set of rules to the specified logical switch. Any existing rules are removed. +func (o *NB) UpdateLogicalSwitchACLRules(ctx context.Context, switchName OVNSwitch, aclRules ...OVNACLRule) error { + operations := []ovsdb.Operation{} + + // Get the logical switch. + ls, err := o.GetLogicalSwitch(ctx, switchName) + if err != nil { + return err + } + // Remove any existing rules assigned to the entity. - args := []string{"clear", "logical_switch", string(switchName), "acls"} + for _, aclUUID := range ls.ACLs { + updateOps, err := o.client.Where(ls).Mutate(ls, ovsModel.Mutation{ + Field: &ls.ACLs, + Mutator: ovsdb.MutateOperationDelete, + Value: []string{aclUUID}, + }) + if err != nil { + return err + } + + operations = append(operations, updateOps...) + } // Add new rules. externalIDs := map[string]string{ ovnExtIDIncusSwitch: string(switchName), } - args = o.aclRuleAddAppendArgs(args, "logical_switch", string(switchName), externalIDs, nil, aclRules...) + createOps, err := o.aclRuleAddOperations(ctx, "logical_switch", string(switchName), externalIDs, nil, aclRules...) + if err != nil { + return err + } + + operations = append(operations, createOps...) - _, err := o.nbctl(args...) + // Apply the database changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err + } + + _, err = ovsdb.CheckOperationResults(resp, operations) if err != nil { return err } @@ -1938,51 +1967,85 @@ func (o *NB) GetLogicalSwitchPortDNS(ctx context.Context, portName OVNSwitchPort return OVNDNSUUID(dnsRecords[0].UUID), dnsName, ips, nil } -// logicalSwitchPortDeleteDNSAppendArgs adds the command arguments to remove DNS records from a switch port. +// logicalSwitchPortDeleteDNSOperations returns a list of ovsdb operations to remove DNS records from a switch port. // If destroyEntry the DNS entry record itself is also removed, otherwise it is just cleared but left in place. -// Returns args with the commands added to it. -func (o *NB) logicalSwitchPortDeleteDNSAppendArgs(args []string, switchName OVNSwitch, dnsUUID OVNDNSUUID, destroyEntry bool) []string { - if len(args) > 0 { - args = append(args, "--") +func (o *NB) logicalSwitchPortDeleteDNSOperations(ctx context.Context, switchName OVNSwitch, dnsUUID OVNDNSUUID, destroyEntry bool) ([]ovsdb.Operation, error) { + operations := []ovsdb.Operation{} + + // Get the DNS entry. + dnsEntry := ovnNB.DNS{ + UUID: string(dnsUUID), + } + + err := o.get(ctx, &dnsEntry) + if err != nil { + return nil, err + } + + // Get the logical switch. + ls, err := o.GetLogicalSwitch(ctx, switchName) + if err != nil { + return nil, err + } + + // Remove from the logical switch. + updateOps, err := o.client.Where(ls).Mutate(ls, ovsModel.Mutation{ + Field: &ls.DNSRecords, + Mutator: ovsdb.MutateOperationDelete, + Value: []string{dnsEntry.UUID}, + }) + if err != nil { + return nil, err } - args = append(args, "remove", "logical_switch", string(switchName), "dns_records", string(dnsUUID), "--") + operations = append(operations, updateOps...) if destroyEntry { - args = append(args, "destroy", "dns", string(dnsUUID)) + deleteOps, err := o.client.Where(&dnsEntry).Delete() + if err != nil { + return nil, err + } + + operations = append(operations, deleteOps...) } else { - args = append(args, "clear", "dns", string(dnsUUID), "records") + dnsEntry.Records = nil + + updateOps, err := o.client.Where(&dnsEntry).Update(&dnsEntry) + if err != nil { + return nil, err + } + + operations = append(operations, updateOps...) } - return args + return operations, nil } -// LogicalSwitchPortDeleteDNS removes DNS records from a switch port. +// DeleteLogicalSwitchPortDNS removes DNS records from a switch port. // If destroyEntry the DNS entry record itself is also removed, otherwise it is just cleared but left in place. -func (o *NB) LogicalSwitchPortDeleteDNS(switchName OVNSwitch, dnsUUID OVNDNSUUID, destroyEntry bool) error { +func (o *NB) DeleteLogicalSwitchPortDNS(ctx context.Context, switchName OVNSwitch, dnsUUID OVNDNSUUID, destroyEntry bool) error { // Remove DNS record association from switch, and remove DNS record entry itself. - _, err := o.nbctl(o.logicalSwitchPortDeleteDNSAppendArgs(nil, switchName, dnsUUID, destroyEntry)...) + operations, err := o.logicalSwitchPortDeleteDNSOperations(ctx, switchName, dnsUUID, destroyEntry) if err != nil { return err } - return nil -} - -// logicalSwitchPortDeleteAppendArgs adds the commands to delete the specified logical switch port. -// Returns args with the commands added to it. -func (o *NB) logicalSwitchPortDeleteAppendArgs(args []string, portName OVNSwitchPort) []string { - if len(args) > 0 { - args = append(args, "--") + // Apply the changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err } - args = append(args, "--if-exists", "lsp-del", string(portName)) + _, err = ovsdb.CheckOperationResults(resp, operations) + if err != nil { + return err + } - return args + return nil } -// DeleteLogicalSwitchPort deletes a named logical switch port. -func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, portName OVNSwitchPort) error { +// logicalSwitchPortDeleteAppendArgs adds the commands to delete the specified logical switch port. +func (o *NB) logicalSwitchPortDeleteOperations(ctx context.Context, switchName OVNSwitch, portName OVNSwitchPort) ([]ovsdb.Operation, error) { operations := []ovsdb.Operation{} // Get the logical switch port. @@ -1994,10 +2057,10 @@ func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, if err != nil { // Logical switch port is already gone. if err == ErrNotFound { - return nil + return nil, nil } - return err + return nil, err } // Remove the port from the switch. @@ -2011,7 +2074,7 @@ func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, Value: []string{logicalSwitchPort.UUID}, }) if err != nil { - return err + return nil, err } operations = append(operations, updateOps...) @@ -2019,11 +2082,22 @@ func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, // Delete the port itself. deleteOps, err := o.client.Where(&logicalSwitchPort).Delete() if err != nil { - return err + return nil, err } operations = append(operations, deleteOps...) + return operations, nil +} + +// DeleteLogicalSwitchPort deletes a named logical switch port. +func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, portName OVNSwitchPort) error { + // Get the delete operations. + operations, err := o.logicalSwitchPortDeleteOperations(ctx, switchName, portName) + if err != nil { + return err + } + // Apply the changes. resp, err := o.client.Transact(ctx, operations...) if err != nil { @@ -2038,25 +2112,48 @@ func (o *NB) DeleteLogicalSwitchPort(ctx context.Context, switchName OVNSwitch, return nil } -// LogicalSwitchPortCleanup deletes the named logical switch port and its associated config. -func (o *NB) LogicalSwitchPortCleanup(portName OVNSwitchPort, switchName OVNSwitch, switchPortGroupName OVNPortGroup, dnsUUID OVNDNSUUID) error { +// CleanupLogicalSwitchPort deletes the named logical switch port and its associated config. +func (o *NB) CleanupLogicalSwitchPort(ctx context.Context, portName OVNSwitchPort, switchName OVNSwitch, switchPortGroupName OVNPortGroup, dnsUUID OVNDNSUUID) error { + operations := []ovsdb.Operation{} + // Remove any existing rules assigned to the entity. - removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(context.TODO(), portName) + removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(ctx, portName) + if err != nil { + return err + } + + deleteOps, err := o.aclRuleDeleteOperations(ctx, "port_group", string(switchPortGroupName), removeACLRuleUUIDs) if err != nil { return err } - args := o.aclRuleDeleteAppendArgs(nil, "port_group", string(switchPortGroupName), removeACLRuleUUIDs) + operations = append(operations, deleteOps...) // Remove logical switch port. - args = o.logicalSwitchPortDeleteAppendArgs(args, portName) + deleteOps, err = o.logicalSwitchPortDeleteOperations(ctx, switchName, portName) + if err != nil { + return err + } + + operations = append(operations, deleteOps...) // Remove DNS records. if dnsUUID != "" { - args = o.logicalSwitchPortDeleteDNSAppendArgs(args, switchName, dnsUUID, false) + deleteOps, err := o.logicalSwitchPortDeleteDNSOperations(ctx, switchName, dnsUUID, false) + if err != nil { + return err + } + + operations = append(operations, deleteOps...) } - _, err = o.nbctl(args...) + // Apply the changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err + } + + _, err = ovsdb.CheckOperationResults(resp, operations) if err != nil { return err } @@ -2530,19 +2627,53 @@ func (o *NB) UpdatePortGroupMembers(ctx context.Context, addMembers map[OVNPortG return nil } -// PortGroupSetACLRules applies a set of rules to the specified port group. Any existing rules are removed. -func (o *NB) PortGroupSetACLRules(portGroupName OVNPortGroup, matchReplace map[string]string, aclRules ...OVNACLRule) error { - // Remove any existing rules assigned to the entity. - args := []string{"clear", "port_group", string(portGroupName), "acls"} +// UpdatePortGroupACLRules applies a set of rules to the specified port group. Any existing rules are removed. +func (o *NB) UpdatePortGroupACLRules(ctx context.Context, portGroupName OVNPortGroup, matchReplace map[string]string, aclRules ...OVNACLRule) error { + operations := []ovsdb.Operation{} + + // Get the port group. + pg := ovnNB.PortGroup{ + Name: string(portGroupName), + } + + err := o.get(ctx, &pg) + if err != nil { + return err + } + + // Remove any existing rules assigned to the port group. + for _, aclUUID := range pg.ACLs { + updateOps, err := o.client.Where(&pg).Mutate(&pg, ovsModel.Mutation{ + Field: &pg.ACLs, + Mutator: ovsdb.MutateOperationDelete, + Value: []string{aclUUID}, + }) + if err != nil { + return err + } + + operations = append(operations, updateOps...) + } // Add new rules. externalIDs := map[string]string{ ovnExtIDIncusPortGroup: string(portGroupName), } - args = o.aclRuleAddAppendArgs(args, "port_group", string(portGroupName), externalIDs, matchReplace, aclRules...) + createOps, err := o.aclRuleAddOperations(ctx, "port_group", string(portGroupName), externalIDs, matchReplace, aclRules...) + if err != nil { + return err + } + + operations = append(operations, createOps...) - _, err := o.nbctl(args...) + // Apply the changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err + } + + _, err = ovsdb.CheckOperationResults(resp, operations) if err != nil { return err } @@ -2550,70 +2681,164 @@ func (o *NB) PortGroupSetACLRules(portGroupName OVNPortGroup, matchReplace map[s return nil } -// aclRuleAddAppendArgs adds the commands to args that add the provided ACL rules to the specified OVN entity. -// Returns args with the ACL rule add commands added to it. -func (o *NB) aclRuleAddAppendArgs(args []string, entityTable string, entityName string, externalIDs map[string]string, matchReplace map[string]string, aclRules ...OVNACLRule) []string { - for i, rule := range aclRules { - if len(args) > 0 { - args = append(args, "--") - } +// aclRuleAddOperations returns the operations to add the provided ACL rules to the specified OVN entity. +func (o *NB) aclRuleAddOperations(ctx context.Context, entityTable string, entityName string, externalIDs map[string]string, matchReplace map[string]string, aclRules ...OVNACLRule) ([]ovsdb.Operation, error) { + operations := []ovsdb.Operation{} + for i, rule := range aclRules { // Perform any replacements requested on the Match string. for find, replace := range matchReplace { rule.Match = strings.ReplaceAll(rule.Match, find, replace) } - // Add command to create ACL rule. - args = append(args, fmt.Sprintf("--id=@id%d", i), "create", "acl", - fmt.Sprintf("action=%s", rule.Action), - fmt.Sprintf("direction=%s", rule.Direction), - fmt.Sprintf("priority=%d", rule.Priority), - fmt.Sprintf("match=%s", strconv.Quote(rule.Match)), - ) + // Add new ACL. + acl := ovnNB.ACL{ + UUID: fmt.Sprintf("acl%d", i), + Action: rule.Action, + Direction: rule.Direction, + Priority: rule.Priority, + Match: rule.Match, + ExternalIDs: map[string]string{}, + } if rule.Log { - args = append(args, "log=true") + acl.Log = true if rule.LogName != "" { - args = append(args, fmt.Sprintf("name=%s", rule.LogName)) + logName := rule.LogName + acl.Name = &logName } } for k, v := range externalIDs { - args = append(args, fmt.Sprintf("external_ids:%s=%s", k, v)) + acl.ExternalIDs[k] = v } - // Add command to assign ACL rule to entity. - args = append(args, "--", "add", entityTable, entityName, "acl", fmt.Sprintf("@id%d", i)) + createOps, err := o.client.Create(&acl) + if err != nil { + return nil, err + } + + operations = append(operations, createOps...) + + // Add ACL rule to entity. + if entityTable == "logical_switch" { + ls := ovnNB.LogicalSwitch{ + Name: entityName, + } + + updateOps, err := o.client.Where(&ls).Mutate(&ls, ovsModel.Mutation{ + Field: &ls.ACLs, + Mutator: ovsdb.MutateOperationInsert, + Value: []string{acl.UUID}, + }) + if err != nil { + return nil, err + } + + operations = append(operations, updateOps...) + } else if entityTable == "port_group" { + pg := ovnNB.PortGroup{ + Name: entityName, + } + + updateOps, err := o.client.Where(&pg).Mutate(&pg, ovsModel.Mutation{ + Field: &pg.ACLs, + Mutator: ovsdb.MutateOperationInsert, + Value: []string{acl.UUID}, + }) + if err != nil { + return nil, err + } + + operations = append(operations, updateOps...) + } else { + return nil, fmt.Errorf("Unsupported entity table %q", entityTable) + } } - return args + return operations, nil } -// aclRuleDeleteAppendArgs adds the commands to args that delete the provided ACL rules from the specified OVN entity. -// Returns args with the ACL rule delete commands added to it. -func (o *NB) aclRuleDeleteAppendArgs(args []string, entityTable string, entityName string, aclRuleUUIDs []string) []string { +// aclRuleDeleteOperations returns the operations that delete the provided ACL rules from the specified OVN entity. +func (o *NB) aclRuleDeleteOperations(ctx context.Context, entityTable string, entityName string, aclRuleUUIDs []string) ([]ovsdb.Operation, error) { + operations := []ovsdb.Operation{} + for _, aclRuleUUID := range aclRuleUUIDs { - if len(args) > 0 { - args = append(args, "--") + // Get the ACL. + acl := ovnNB.ACL{ + UUID: aclRuleUUID, + } + + err := o.get(ctx, &acl) + if err != nil { + return nil, err + } + + // Delete the ACL. + deleteOps, err := o.client.Where(&acl).Delete() + if err != nil { + return nil, err } - args = append(args, "remove", entityTable, string(entityName), "acl", aclRuleUUID) + operations = append(operations, deleteOps...) + + // Remove ACL rule from entity. + if entityTable == "logical_switch" { + ls := ovnNB.LogicalSwitch{ + Name: entityName, + } + + updateOps, err := o.client.Where(&ls).Mutate(&ls, ovsModel.Mutation{ + Field: &ls.ACLs, + Mutator: ovsdb.MutateOperationDelete, + Value: []string{acl.UUID}, + }) + if err != nil { + return nil, err + } + + operations = append(operations, updateOps...) + } else if entityTable == "port_group" { + pg := ovnNB.PortGroup{ + Name: entityName, + } + + updateOps, err := o.client.Where(&pg).Mutate(&pg, ovsModel.Mutation{ + Field: &pg.ACLs, + Mutator: ovsdb.MutateOperationDelete, + Value: []string{acl.UUID}, + }) + if err != nil { + return nil, err + } + + operations = append(operations, updateOps...) + } else { + return nil, fmt.Errorf("Unsupported entity table %q", entityTable) + } } - return args + return operations, nil } -// PortGroupPortSetACLRules applies a set of rules for the logical switch port in the specified port group. +// UpdatePortGroupPortACLRules applies a set of rules for the logical switch port in the specified port group. // Any existing rules for that logical switch port in the port group are removed. -func (o *NB) PortGroupPortSetACLRules(portGroupName OVNPortGroup, portName OVNSwitchPort, aclRules ...OVNACLRule) error { +func (o *NB) UpdatePortGroupPortACLRules(ctx context.Context, portGroupName OVNPortGroup, portName OVNSwitchPort, aclRules ...OVNACLRule) error { + operations := []ovsdb.Operation{} + // Remove any existing rules assigned to the entity. - removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(context.TODO(), portName) + removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(ctx, portName) + if err != nil { + return err + } + + deleteOps, err := o.aclRuleDeleteOperations(ctx, "port_group", string(portGroupName), removeACLRuleUUIDs) if err != nil { return err } - args := o.aclRuleDeleteAppendArgs(nil, "port_group", string(portGroupName), removeACLRuleUUIDs) + operations = append(operations, deleteOps...) // Add new rules. externalIDs := map[string]string{ @@ -2621,9 +2846,20 @@ func (o *NB) PortGroupPortSetACLRules(portGroupName OVNPortGroup, portName OVNSw ovnExtIDIncusSwitchPort: string(portName), } - args = o.aclRuleAddAppendArgs(args, "port_group", string(portGroupName), externalIDs, nil, aclRules...) + createOps, err := o.aclRuleAddOperations(ctx, "port_group", string(portGroupName), externalIDs, nil, aclRules...) + if err != nil { + return err + } + + operations = append(operations, createOps...) - _, err = o.nbctl(args...) + // Apply the changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err + } + + _, err = ovsdb.CheckOperationResults(resp, operations) if err != nil { return err } @@ -2631,21 +2867,28 @@ func (o *NB) PortGroupPortSetACLRules(portGroupName OVNPortGroup, portName OVNSw return nil } -// PortGroupPortClearACLRules clears any rules assigned to the logical switch port in the specified port group. -func (o *NB) PortGroupPortClearACLRules(portGroupName OVNPortGroup, portName OVNSwitchPort) error { +// ClearPortGroupPortACLRules clears any rules assigned to the logical switch port in the specified port group. +func (o *NB) ClearPortGroupPortACLRules(ctx context.Context, portGroupName OVNPortGroup, portName OVNSwitchPort) error { // Remove any existing rules assigned to the entity. - removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(context.TODO(), portName) + removeACLRuleUUIDs, err := o.logicalSwitchPortACLRules(ctx, portName) if err != nil { return err } - args := o.aclRuleDeleteAppendArgs(nil, "port_group", string(portGroupName), removeACLRuleUUIDs) + operations, err := o.aclRuleDeleteOperations(ctx, "port_group", string(portGroupName), removeACLRuleUUIDs) + if err != nil { + return err + } - if len(args) > 0 { - _, err = o.nbctl(args...) - if err != nil { - return err - } + // Apply the changes. + resp, err := o.client.Transact(ctx, operations...) + if err != nil { + return err + } + + _, err = ovsdb.CheckOperationResults(resp, operations) + if err != nil { + return err } return nil