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

chore(source): code cleanup #5189

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
141e87b
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 6, 2025
a3395cc
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 7, 2025
fc24c55
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 11, 2025
6e691be
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 13, 2025
32f0eeb
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 14, 2025
34b469a
Merge branch 'kubernetes-sigs:master' into master
ivankatliarchuk Mar 16, 2025
37a6621
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
eba1dc6
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
ac3cb68
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
f9d399e
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
85902d0
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
7e7f7ff
chore(source): code cleanup
ivankatliarchuk Mar 17, 2025
361631d
chore(source): code cleanup
ivankatliarchuk Mar 18, 2025
fcda79f
chore(source): code cleanup
ivankatliarchuk Mar 18, 2025
b2795e0
chore(source): code cleanup
ivankatliarchuk Mar 19, 2025
ba698e6
Merge branch 'kubernetes-sigs:master' into feat-code-cleanup-0
ivankatliarchuk Mar 24, 2025
f285cb1
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
4910d30
Merge remote-tracking branch 'refs/remotes/origin/feat-code-cleanup-0…
ivankatliarchuk Mar 28, 2025
d2dc8c6
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
b3ce318
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
62554cb
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
50bf165
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
9bd6e1d
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
b16775d
chore(source): code cleanup
ivankatliarchuk Mar 28, 2025
505557a
Merge branch 'master' into feat-code-cleanup-0
ivankatliarchuk Apr 1, 2025
bcb2dd8
chore(source): code cleanup
ivankatliarchuk Apr 1, 2025
631c789
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
3c09340
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
d88e9b2
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
1d1ac2e
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
87a66f9
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
7bbc566
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
6fcc697
chore(source): code cleanup
ivankatliarchuk Apr 2, 2025
e7435c7
chore(source): code cleanup
ivankatliarchuk Apr 6, 2025
7e1147c
chore(source): code cleanup
ivankatliarchuk Apr 6, 2025
2b703e5
chore(source): code cleanup
ivankatliarchuk Apr 6, 2025
87f34fd
chore(source): code cleanup
ivankatliarchuk Apr 6, 2025
807f2e8
chore(source): code cleanup
ivankatliarchuk Apr 6, 2025
86b92e1
Merge branch 'master' into feat-code-cleanup-0
ivankatliarchuk Apr 9, 2025
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
33 changes: 33 additions & 0 deletions source/annotations/annotations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
Copyright 2025 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package annotations

import (
"math"
)

const (
// CloudflareProxiedKey The annotation used for determining if traffic will go through Cloudflare
CloudflareProxiedKey = "external-dns.alpha.kubernetes.io/cloudflare-proxied"
CloudflareCustomHostnameKey = "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname"

SetIdentifierKey = "external-dns.alpha.kubernetes.io/set-identifier"
AliasAnnotationKey = "external-dns.alpha.kubernetes.io/alias"

TargetAnnotationKey = "external-dns.alpha.kubernetes.io/target"

TtlAnnotationKey = "external-dns.alpha.kubernetes.io/ttl"
ttlMinimum = 1
ttlMaximum = math.MaxInt32
)
102 changes: 102 additions & 0 deletions source/annotations/helpers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
Copyright 2025 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package annotations

import (
"strconv"
"strings"
"time"

log "github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"

"sigs.k8s.io/external-dns/endpoint"
)

func hasAliasFromAnnotations(annotations map[string]string) bool {
aliasAnnotation, exists := annotations[AliasAnnotationKey]
return exists && aliasAnnotation == "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

In Go we use "ok" as var name in these cases, please change all of them.
exists -> ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

// TTLFromAnnotations TODO: copied from source.go. Refactor to avoid duplication.
// TTLFromAnnotations extracts the TTL from the annotations of the given resource.
func TTLFromAnnotations(annotations map[string]string, resource string) endpoint.TTL {
ttlNotConfigured := endpoint.TTL(0)
ttlAnnotation, exists := annotations[TtlAnnotationKey]
if !exists {
return ttlNotConfigured
}
ttlValue, err := parseTTL(ttlAnnotation)
if err != nil {
log.Warnf("%s: \"%v\" is not a valid TTL value: %v", resource, ttlAnnotation, err)
return ttlNotConfigured
}
if ttlValue < ttlMinimum || ttlValue > ttlMaximum {
log.Warnf("TTL value %q must be between [%d, %d]", ttlValue, ttlMinimum, ttlMaximum)
return ttlNotConfigured
}
return endpoint.TTL(ttlValue)
}

// parseTTL parses TTL from string, returning duration in seconds.
// parseTTL supports both integers like "600" and durations based
// on Go Duration like "10m", hence "600" and "10m" represent the same value.
//
// Note: for durations like "1.5s" the fraction is omitted (resulting in 1 second
// for the example).
func parseTTL(s string) (ttlSeconds int64, err error) {
ttlDuration, errDuration := time.ParseDuration(s)
if errDuration != nil {
ttlInt, err := strconv.ParseInt(s, 10, 64)
if err != nil {
return 0, errDuration
}
return ttlInt, nil
}

return int64(ttlDuration.Seconds()), nil
}

// ParseAnnotationFilter parses an annotation filter string into a labels.Selector.
// Returns nil if the annotation filter is invalid.
func ParseAnnotationFilter(annotationFilter string) (labels.Selector, error) {
labelSelector, err := metav1.ParseToLabelSelector(annotationFilter)
if err != nil {
return nil, err
}
selector, err := metav1.LabelSelectorAsSelector(labelSelector)
if err != nil {
return nil, err
}
return selector, nil
}

// TargetsFromTargetAnnotation gets endpoints from optional "target" annotation.
// Returns empty endpoints array if none are found.
func TargetsFromTargetAnnotation(annotations map[string]string) endpoint.Targets {
var targets endpoint.Targets

// Get the desired hostname of the ingress from the annotation.
targetAnnotation, exists := annotations[TargetAnnotationKey]
if exists && targetAnnotation != "" {
// splits the hostname annotation and removes the trailing periods
targetsList := strings.Split(strings.Replace(targetAnnotation, " ", "", -1), ",")
for _, targetHostname := range targetsList {
targetHostname = strings.TrimSuffix(targetHostname, ".")
targets = append(targets, targetHostname)
}
}
return targets
}
189 changes: 189 additions & 0 deletions source/annotations/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
/*
Copyright 2025 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package annotations

import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/external-dns/endpoint"
)

func TestParseAnnotationFilter(t *testing.T) {
tests := []struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI, so does not need to be changed: you can also omit the "tests" var and start with the for loop and have an inline struct definition. I think it looks like this (out of my head without parser or compiler):

for _, tt := range []struct{
   name string
   ...
  }{
    name: "myName",
    ...
  }, {
     name: "anotherName",
     ....
  }, {
     name: "lastName",
     ....
  }} {
    t.Run(tt.name, func(t *testing.T) {
      // body
    })
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not going to touch it, not because against, mainly due to consistency. There is not a single test with this design at the moment
Screenshot 2025-03-28 at 21 48 40

name string
annotationFilter string
expectedSelector labels.Selector
expectError bool
}{
{
name: "valid annotation filter",
annotationFilter: "key1=value1,key2=value2",
expectedSelector: labels.Set{"key1": "value1", "key2": "value2"}.AsSelector(),
expectError: false,
},
{
name: "invalid annotation filter",
annotationFilter: "key1==value1",
expectedSelector: labels.Set{"key1": "value1"}.AsSelector(),
expectError: false,
},
{
name: "empty annotation filter",
annotationFilter: "",
expectedSelector: labels.Set{}.AsSelector(),
expectError: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
selector, err := ParseAnnotationFilter(tt.annotationFilter)
if tt.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expectedSelector, selector)
}
})
}
}

func TestTargetsFromTargetAnnotation(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
expected endpoint.Targets
}{
{
name: "no target annotation",
annotations: map[string]string{},
expected: endpoint.Targets(nil),
},
{
name: "single target annotation",
annotations: map[string]string{
TargetAnnotationKey: "example.com",
},
expected: endpoint.Targets{"example.com"},
},
{
name: "multiple target annotations",
annotations: map[string]string{
TargetAnnotationKey: "example.com,example.org",
},
expected: endpoint.Targets{"example.com", "example.org"},
},
{
name: "target annotation with trailing periods",
annotations: map[string]string{
TargetAnnotationKey: "example.com.,example.org.",
},
expected: endpoint.Targets{"example.com", "example.org"},
},
{
name: "target annotation with spaces",
annotations: map[string]string{
TargetAnnotationKey: " example.com , example.org ",
},
expected: endpoint.Targets{"example.com", "example.org"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := TargetsFromTargetAnnotation(tt.annotations)
assert.Equal(t, tt.expected, result)
})
}
}

func TestTTLFromAnnotations(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
resource string
expectedTTL endpoint.TTL
}{
{
name: "no TTL annotation",
annotations: map[string]string{},
resource: "test-resource",
expectedTTL: endpoint.TTL(0),
},
{
name: "valid TTL annotation",
annotations: map[string]string{
TtlAnnotationKey: "600",
},
resource: "test-resource",
expectedTTL: endpoint.TTL(600),
},
{
name: "invalid TTL annotation",
annotations: map[string]string{
TtlAnnotationKey: "invalid",
},
resource: "test-resource",
expectedTTL: endpoint.TTL(0),
},
{
name: "TTL annotation out of range",
annotations: map[string]string{
TtlAnnotationKey: "999999",
},
resource: "test-resource",
expectedTTL: endpoint.TTL(999999),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ttl := TTLFromAnnotations(tt.annotations, tt.resource)
assert.Equal(t, tt.expectedTTL, ttl)
})
}
}

func TestGetAliasFromAnnotations(t *testing.T) {
tests := []struct {
name string
annotations map[string]string
expected bool
}{
{
name: "alias annotation exists and is true",
annotations: map[string]string{AliasAnnotationKey: "true"},
expected: true,
},
{
name: "alias annotation exists and is false",
annotations: map[string]string{AliasAnnotationKey: "false"},
expected: false,
},
{
name: "alias annotation does not exist",
annotations: map[string]string{},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := hasAliasFromAnnotations(tt.annotations)
assert.Equal(t, tt.expected, result)
})
}
}
Loading
Loading