Skip to content

Commit

Permalink
fix: correct diffs when comparing objects with empty tags (#985)
Browse files Browse the repository at this point in the history
Right now, when some declarative configuration having empty tags
on any of its entities is deployed, subsequent `diff` / `sync`
will show misleading results:

```
$ cat kong.yaml
_format_version: "3.0"
services:
  - name: foo
    protocol: http
    host: foo
    port: 8000
    tags: []

$ deck sync
creating service foo
Summary:
  Created: 1
  Updated: 0
  Deleted: 0

$ deck sync
updating service foo  {
   "connect_timeout": 60000,
   "enabled": true,
   "host": "foo",
   "id": "afadd2ed-2bcd-49fc-9000-adb58cd11b50",
   "name": "foo",
   "port": 8000,
   "protocol": "http",
   "read_timeout": 60000,
   "retries": 5,
   "write_timeout": 60000
 }

Summary:
  Created: 0
  Updated: 1
  Deleted: 0
```

The second `deck sync` run is showing an update without an actual diff.
This is due to the way empty tags are treated internally in decK.

This commit fixes this defect.
  • Loading branch information
GGabriele committed Jul 27, 2023
1 parent 24ef372 commit aee76dd
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 0 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@
- [v0.2.0](#v020)
- [v0.1.0](#v010)

## Unreleased

> Release date: TBD
### Fixes

- Avoid misleading diffs when configuration file has empty tags.
[#985](https://github.com/Kong/deck/pull/985)


## [v1.24.0]

> Release date: 2023/07/24
Expand Down
119 changes: 119 additions & 0 deletions state/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@ func (s1 *Service) EqualWithOpts(s2 *Service,
s1Copy := s1.Service.DeepCopy()
s2Copy := s2.Service.DeepCopy()

if len(s1Copy.Tags) == 0 {
s1Copy.Tags = nil
}
if len(s2Copy.Tags) == 0 {
s2Copy.Tags = nil
}

// Cassandra can sometimes mess up tag order, but tag order doesn't actually matter: tags are sets
// even though we represent them with slices. Sort before comparison to avoid spurious diff detection.
sort.Slice(s1Copy.Tags, func(i, j int) bool { return *(s1Copy.Tags[i]) < *(s1Copy.Tags[j]) })
Expand Down Expand Up @@ -153,6 +160,13 @@ func (r1 *Route) EqualWithOpts(r2 *Route, ignoreID,
r1Copy := r1.Route.DeepCopy()
r2Copy := r2.Route.DeepCopy()

if len(r1Copy.Tags) == 0 {
r1Copy.Tags = nil
}
if len(r2Copy.Tags) == 0 {
r2Copy.Tags = nil
}

sort.Slice(r1Copy.Tags, func(i, j int) bool { return *(r1Copy.Tags[i]) < *(r1Copy.Tags[j]) })
sort.Slice(r2Copy.Tags, func(i, j int) bool { return *(r2Copy.Tags[i]) < *(r2Copy.Tags[j]) })

Expand Down Expand Up @@ -216,6 +230,13 @@ func (u1 *Upstream) EqualWithOpts(u2 *Upstream,
u1Copy := u1.Upstream.DeepCopy()
u2Copy := u2.Upstream.DeepCopy()

if len(u1Copy.Tags) == 0 {
u1Copy.Tags = nil
}
if len(u2Copy.Tags) == 0 {
u2Copy.Tags = nil
}

sort.Slice(u1Copy.Tags, func(i, j int) bool { return *(u1Copy.Tags[i]) < *(u1Copy.Tags[j]) })
sort.Slice(u2Copy.Tags, func(i, j int) bool { return *(u2Copy.Tags[i]) < *(u2Copy.Tags[j]) })

Expand Down Expand Up @@ -270,6 +291,13 @@ func (t1 *Target) EqualWithOpts(t2 *Target, ignoreID,
t1Copy := t1.Target.DeepCopy()
t2Copy := t2.Target.DeepCopy()

if len(t1Copy.Tags) == 0 {
t1Copy.Tags = nil
}
if len(t2Copy.Tags) == 0 {
t2Copy.Tags = nil
}

sort.Slice(t1Copy.Tags, func(i, j int) bool { return *(t1Copy.Tags[i]) < *(t1Copy.Tags[j]) })
sort.Slice(t2Copy.Tags, func(i, j int) bool { return *(t2Copy.Tags[i]) < *(t2Copy.Tags[j]) })

Expand Down Expand Up @@ -324,6 +352,13 @@ func (c1 *Certificate) EqualWithOpts(c2 *Certificate,
c1Copy := c1.Certificate.DeepCopy()
c2Copy := c2.Certificate.DeepCopy()

if len(c1Copy.Tags) == 0 {
c1Copy.Tags = nil
}
if len(c2Copy.Tags) == 0 {
c2Copy.Tags = nil
}

sort.Slice(c1Copy.Tags, func(i, j int) bool { return *(c1Copy.Tags[i]) < *(c1Copy.Tags[j]) })
sort.Slice(c2Copy.Tags, func(i, j int) bool { return *(c2Copy.Tags[i]) < *(c2Copy.Tags[j]) })

Expand Down Expand Up @@ -374,6 +409,13 @@ func (s1 *SNI) EqualWithOpts(s2 *SNI, ignoreID,
s1Copy := s1.SNI.DeepCopy()
s2Copy := s2.SNI.DeepCopy()

if len(s1Copy.Tags) == 0 {
s1Copy.Tags = nil
}
if len(s2Copy.Tags) == 0 {
s2Copy.Tags = nil
}

sort.Slice(s1Copy.Tags, func(i, j int) bool { return *(s1Copy.Tags[i]) < *(s1Copy.Tags[j]) })
sort.Slice(s2Copy.Tags, func(i, j int) bool { return *(s2Copy.Tags[i]) < *(s2Copy.Tags[j]) })

Expand Down Expand Up @@ -452,6 +494,13 @@ func (p1 *Plugin) EqualWithOpts(p2 *Plugin, ignoreID,
p1Copy := p1.Plugin.DeepCopy()
p2Copy := p2.Plugin.DeepCopy()

if len(p1Copy.Tags) == 0 {
p1Copy.Tags = nil
}
if len(p2Copy.Tags) == 0 {
p2Copy.Tags = nil
}

sort.Slice(p1Copy.Tags, func(i, j int) bool { return *(p1Copy.Tags[i]) < *(p1Copy.Tags[j]) })
sort.Slice(p2Copy.Tags, func(i, j int) bool { return *(p2Copy.Tags[i]) < *(p2Copy.Tags[j]) })

Expand Down Expand Up @@ -528,6 +577,13 @@ func (c1 *Consumer) EqualWithOpts(c2 *Consumer,
c1Copy := c1.Consumer.DeepCopy()
c2Copy := c2.Consumer.DeepCopy()

if len(c1Copy.Tags) == 0 {
c1Copy.Tags = nil
}
if len(c2Copy.Tags) == 0 {
c2Copy.Tags = nil
}

sort.Slice(c1Copy.Tags, func(i, j int) bool { return *(c1Copy.Tags[i]) < *(c1Copy.Tags[j]) })
sort.Slice(c2Copy.Tags, func(i, j int) bool { return *(c2Copy.Tags[i]) < *(c2Copy.Tags[j]) })

Expand Down Expand Up @@ -778,6 +834,13 @@ func (k1 *KeyAuth) EqualWithOpts(k2 *KeyAuth, ignoreID,
k1Copy := k1.KeyAuth.DeepCopy()
k2Copy := k2.KeyAuth.DeepCopy()

if len(k1Copy.Tags) == 0 {
k1Copy.Tags = nil
}
if len(k2Copy.Tags) == 0 {
k2Copy.Tags = nil
}

sort.Slice(k1Copy.Tags, func(i, j int) bool { return *(k1Copy.Tags[i]) < *(k1Copy.Tags[j]) })
sort.Slice(k2Copy.Tags, func(i, j int) bool { return *(k2Copy.Tags[i]) < *(k2Copy.Tags[j]) })

Expand Down Expand Up @@ -856,6 +919,13 @@ func (h1 *HMACAuth) EqualWithOpts(h2 *HMACAuth, ignoreID,
h1Copy := h1.HMACAuth.DeepCopy()
h2Copy := h2.HMACAuth.DeepCopy()

if len(h1Copy.Tags) == 0 {
h1Copy.Tags = nil
}
if len(h2Copy.Tags) == 0 {
h2Copy.Tags = nil
}

sort.Slice(h1Copy.Tags, func(i, j int) bool { return *(h1Copy.Tags[i]) < *(h1Copy.Tags[j]) })
sort.Slice(h2Copy.Tags, func(i, j int) bool { return *(h2Copy.Tags[i]) < *(h2Copy.Tags[j]) })

Expand Down Expand Up @@ -934,6 +1004,13 @@ func (j1 *JWTAuth) EqualWithOpts(j2 *JWTAuth, ignoreID,
j1Copy := j1.JWTAuth.DeepCopy()
j2Copy := j2.JWTAuth.DeepCopy()

if len(j1Copy.Tags) == 0 {
j1Copy.Tags = nil
}
if len(j2Copy.Tags) == 0 {
j2Copy.Tags = nil
}

sort.Slice(j1Copy.Tags, func(i, j int) bool { return *(j1Copy.Tags[i]) < *(j1Copy.Tags[j]) })
sort.Slice(j2Copy.Tags, func(i, j int) bool { return *(j2Copy.Tags[i]) < *(j2Copy.Tags[j]) })

Expand Down Expand Up @@ -1012,6 +1089,13 @@ func (b1 *BasicAuth) EqualWithOpts(b2 *BasicAuth, ignoreID,
b1Copy := b1.BasicAuth.DeepCopy()
b2Copy := b2.BasicAuth.DeepCopy()

if len(b1Copy.Tags) == 0 {
b1Copy.Tags = nil
}
if len(b2Copy.Tags) == 0 {
b2Copy.Tags = nil
}

sort.Slice(b1Copy.Tags, func(i, j int) bool { return *(b1Copy.Tags[i]) < *(b1Copy.Tags[j]) })
sort.Slice(b2Copy.Tags, func(i, j int) bool { return *(b2Copy.Tags[i]) < *(b2Copy.Tags[j]) })

Expand Down Expand Up @@ -1094,6 +1178,13 @@ func (b1 *ACLGroup) EqualWithOpts(b2 *ACLGroup, ignoreID,
b1Copy := b1.ACLGroup.DeepCopy()
b2Copy := b2.ACLGroup.DeepCopy()

if len(b1Copy.Tags) == 0 {
b1Copy.Tags = nil
}
if len(b2Copy.Tags) == 0 {
b2Copy.Tags = nil
}

sort.Slice(b1Copy.Tags, func(i, j int) bool { return *(b1Copy.Tags[i]) < *(b1Copy.Tags[j]) })
sort.Slice(b2Copy.Tags, func(i, j int) bool { return *(b2Copy.Tags[i]) < *(b2Copy.Tags[j]) })

Expand Down Expand Up @@ -1154,6 +1245,13 @@ func (c1 *CACertificate) EqualWithOpts(c2 *CACertificate,
c1Copy := c1.CACertificate.DeepCopy()
c2Copy := c2.CACertificate.DeepCopy()

if len(c1Copy.Tags) == 0 {
c1Copy.Tags = nil
}
if len(c2Copy.Tags) == 0 {
c2Copy.Tags = nil
}

sort.Slice(c1Copy.Tags, func(i, j int) bool { return *(c1Copy.Tags[i]) < *(c1Copy.Tags[j]) })
sort.Slice(c2Copy.Tags, func(i, j int) bool { return *(c2Copy.Tags[i]) < *(c2Copy.Tags[j]) })

Expand Down Expand Up @@ -1195,6 +1293,13 @@ func (k1 *Oauth2Credential) EqualWithOpts(k2 *Oauth2Credential, ignoreID,
k1Copy := k1.Oauth2Credential.DeepCopy()
k2Copy := k2.Oauth2Credential.DeepCopy()

if len(k1Copy.Tags) == 0 {
k1Copy.Tags = nil
}
if len(k2Copy.Tags) == 0 {
k2Copy.Tags = nil
}

sort.Slice(k1Copy.Tags, func(i, j int) bool { return *(k1Copy.Tags[i]) < *(k1Copy.Tags[j]) })
sort.Slice(k2Copy.Tags, func(i, j int) bool { return *(k2Copy.Tags[i]) < *(k2Copy.Tags[j]) })

Expand Down Expand Up @@ -1273,6 +1378,13 @@ func (b1 *MTLSAuth) EqualWithOpts(b2 *MTLSAuth, ignoreID,
b1Copy := b1.MTLSAuth.DeepCopy()
b2Copy := b2.MTLSAuth.DeepCopy()

if len(b1Copy.Tags) == 0 {
b1Copy.Tags = nil
}
if len(b2Copy.Tags) == 0 {
b2Copy.Tags = nil
}

sort.Slice(b1Copy.Tags, func(i, j int) bool { return *(b1Copy.Tags[i]) < *(b1Copy.Tags[j]) })
sort.Slice(b2Copy.Tags, func(i, j int) bool { return *(b2Copy.Tags[i]) < *(b2Copy.Tags[j]) })

Expand Down Expand Up @@ -1454,6 +1566,13 @@ func (v1 *Vault) EqualWithOpts(v2 *Vault, ignoreID, ignoreTS bool) bool {
v1Copy := v1.Vault.DeepCopy()
v2Copy := v2.Vault.DeepCopy()

if len(v1Copy.Tags) == 0 {
v1Copy.Tags = nil
}
if len(v2Copy.Tags) == 0 {
v2Copy.Tags = nil
}

sort.Slice(v1Copy.Tags, func(i, j int) bool { return *(v1Copy.Tags[i]) < *(v1Copy.Tags[j]) })
sort.Slice(v2Copy.Tags, func(i, j int) bool { return *(v2Copy.Tags[i]) < *(v2Copy.Tags[j]) })

Expand Down

0 comments on commit aee76dd

Please sign in to comment.