Skip to content

Commit

Permalink
Trim ACL names according to RFC1123
Browse files Browse the repository at this point in the history
We didn't consider this tiny hack that we do
https://github.com/openshift/ovn-kubernetes/blob/44ad75466e486cce605e39513a3ecd9e0b306e7d/go-controller/pkg/libovsdbops/acl.go#L60
there when we wrote openshift#1259
and unit tests don't actually scream loudly for longer names.

Without this PR we break users who have namespace names
longer than 45 characters.

NOTE: In 4.11
openshift@44ad754#diff-5b9331449265f93da0d6fac90800eb68b7cb28f72b3f55eb01ad7026fc2b6089R68
is missing so we are directly using libovsdbops.BuildACL.

Signed-off-by: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Co-authored-by: Patryk Diak <pdiak@redhat.com>
(cherry picked from commit b10ed7b)
(cherry picked from commit 54381b0)
  • Loading branch information
tssurya committed Sep 27, 2022
1 parent d2b3afa commit 18fe8be
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
17 changes: 14 additions & 3 deletions go-controller/pkg/ovn/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (oc *Controller) updateStaleDefaultDenyACLNames(npType knet.PolicyType, gre
return item.ExternalIDs[defaultDenyPolicyTypeACLExtIdKey] == string(npType) && // default-deny-policy-type:Egress or default-deny-policy-type:Ingress
strings.Contains(item.Match, gressSuffix) && // Match:inport == @ablah80448_egressDefaultDeny or Match:inport == @ablah80448_ingressDefaultDeny
!strings.Contains(*item.Name, arpAllowPolicySuffix) && // != name: namespace_ARPallowPolicy
!strings.Contains(*item.Name, gressSuffix) // filter out already converted ACLs
!strings.Contains(*item.Name, gressSuffix) // filter out already converted ACLs (we still continue to pick the cases where names were >45 chars)
}
gressACLs, err := libovsdbops.FindACLsWithPredicate(oc.nbClient, p)
if err != nil {
Expand All @@ -145,8 +145,19 @@ func (oc *Controller) updateStaleDefaultDenyACLNames(npType knet.PolicyType, gre
return err
}
}
aclList[0].Name = &newName
err := libovsdbops.CreateOrUpdateACLs(oc.nbClient, aclList[0])
newACL := libovsdbops.BuildACL(
newName, // this is the only thing we need to change, keep the rest same
aclList[0].Direction,
aclList[0].Priority,
aclList[0].Match,
aclList[0].Action,
*aclList[0].Meter,
*aclList[0].Severity,
aclList[0].Log,
aclList[0].ExternalIDs,
aclList[0].Options,
)
err := libovsdbops.CreateOrUpdateACLs(oc.nbClient, newACL)
if err != nil {
return fmt.Errorf("cannot update old NetworkPolicy ACLs for namespace %s: %v", namespace, err)
}
Expand Down
4 changes: 2 additions & 2 deletions go-controller/pkg/ovn/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ var _ = ginkgo.Describe("OVN NetworkPolicy Operations", func() {

// ACL3: leftover default deny ACL egress with old name (namespace_policyname)
leftOverACL3FromUpgrade := libovsdbops.BuildACL(
"leftover1"+"_"+networkPolicy2.Name,
"youknownothingjonsnowyouknownothingjonsnowyouknownothingjonsnow"+"_"+networkPolicy2.Name,
nbdb.ACLDirectionFromLport,
types.DefaultDenyPriority,
"inport == @"+pgHash+"_"+egressDenyPG,
Expand Down Expand Up @@ -1934,7 +1934,7 @@ var _ = ginkgo.Describe("OVN NetworkPolicy Operations", func() {
"apply-after-lb": "true",
}
leftOverACL3FromUpgrade.Options = egressOptions
newDefaultDenyEgressACLName := "leftover1_" + egressDefaultDenySuffix
newDefaultDenyEgressACLName := "youknownothingjonsnowyouknownothingjonsnowyouknownothingjonsnow" // trims it according to RFC1123
newDefaultDenyIngressACLName := "leftover1_" + ingressDefaultDenySuffix
leftOverACL3FromUpgrade.Name = &newDefaultDenyEgressACLName
leftOverACL4FromUpgrade.Name = &newDefaultDenyIngressACLName
Expand Down

0 comments on commit 18fe8be

Please sign in to comment.