Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 3 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[Makefile]
# https://github.com/editorconfig/editorconfig-core-go/blob/master/.editorconfig
[{Makefile,go.mod,go.sum,*.go,.gitmodules}]
indent_style = tab
indent_size = 4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Current indent_size on all go files in master is 2.
Why would we want to change it to 4 ? It would force to update all codebase and force all PRs to rebase.

side note: there are no gitmodules in this project

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed submodules.
I've added gofmt -l -s -w . to go-lint docs https://pkg.go.dev/cmd/gofmt

I only found this few files that
Screenshot 2025-02-17 at 08 31 56
have issues with intendations, rest of them are fine

Copy link
Member Author

@ivankatliarchuk ivankatliarchuk Feb 17, 2025

Choose a reason for hiding this comment

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

Shell we add gofmt validation to github actions?

Copy link
Member Author

@ivankatliarchuk ivankatliarchuk Feb 17, 2025

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Current indent_size on all go files in master is 2. Why would we want to change it to 4 ? It would force to update all codebase and force all PRs to rebase.

side note: there are no gitmodules in this project

Golang is 4 tabs intendation, which is set across the project.

The proper Go style way to do this is to configure your editor's display of tabs instead.

Is it shows 2 spaces in your editor?

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I checked yesterday, it was displaying 2 spaces.
Today, it display 4 spaces, as you say 😅 .

Shall we add gofmt validation to github actions?

Yes, it sounds good 👍 .

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice !

7 changes: 7 additions & 0 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ jobs:
with:
go-version-file: go.mod

# https://github.com/golangci/golangci-lint-action?tab=readme-ov-file#verify
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wdyt about adding verify: true in current golangci lint action instead of creating a new one ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed. First impression was, that it does not support both operation, but looks like it does. Validated here https://github.com/golangci/golangci-lint-action/blob/3b4f037d0e94e85d98f9824ef87b2dc32d53fbd5/src/run.ts#L140

- name: Verify linter configuration
uses: golangci/golangci-lint-action@v6
with:
verify: true
version: v1.63

- name: Lint go code
uses: golangci/golangci-lint-action@v6
with:
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ endif
golangci-lint:
@command -v golangci-lint > /dev/null || curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.63.4

#? golangci-lint-verify: Verify golangci-lint configuration
golangci-lint-verify: golangci-lint
@golangci-lint config verify

#? go-lint: Run the golangci-lint tool
.PHONY: go-lint
go-lint: golangci-lint
Expand Down
2 changes: 1 addition & 1 deletion docs/contributing/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This folder contains developer documentation.

When you are ready to contribute, you can select issue at [Good First Issues](https://github.com/kubernetes-sigs/external-dns/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22help%20wanted%22).

To get started see: [dev-guide.md](devguide.md).
To get started see: [dev-guide.md](dev-guide.md).

> Note; when new feature/fix is ready, consider also to provide a way to test this manually with manifests and kubectl commands

Expand Down
2 changes: 1 addition & 1 deletion provider/aws/aws_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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
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,
Expand Down
4 changes: 2 additions & 2 deletions provider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1919,15 +1919,15 @@ func TestAWSCanonicalHostedZoneNotExist(t *testing.T) {

func BenchmarkTestAWSCanonicalHostedZone(b *testing.B) {
for i := 0; i < b.N; i++ {
for suffix, _ := range canonicalHostedZones {
for suffix := range canonicalHostedZones {
_ = canonicalHostedZone(fmt.Sprintf("foo.%s", suffix))
}
}
}

func BenchmarkTestAWSNonCanonicalHostedZone(b *testing.B) {
for i := 0; i < b.N; i++ {
for _, _ = range canonicalHostedZones {
for range canonicalHostedZones {
_ = canonicalHostedZone("extremely.long.zone-2.ext.dns.test.zone.non.canonical.example.com")
}
}
Expand Down
170 changes: 85 additions & 85 deletions provider/aws/aws_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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
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,
Expand All @@ -17,132 +17,132 @@ limitations under the License.
package aws

import (
"context"
"os"
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/service/route53"
route53types "github.com/aws/aws-sdk-go-v2/service/route53/types"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/provider"
"context"
"os"
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/service/route53"
route53types "github.com/aws/aws-sdk-go-v2/service/route53/types"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/provider"
)

type HostedZones struct {
Zones []*HostedZone `yaml:"zones"`
Zones []*HostedZone `yaml:"zones"`
}

type HostedZone struct {
Name string
ID string
Tags []route53types.Tag `yaml:"tags"`
Name string
ID string
Tags []route53types.Tag `yaml:"tags"`
}

var _ Route53API = &Route53APIFixtureStub{}

type Route53APIFixtureStub struct {
zones map[string]*route53types.HostedZone
zoneTags map[string][]route53types.Tag
calls map[string]int
zones map[string]*route53types.HostedZone
zoneTags map[string][]route53types.Tag
calls map[string]int
}

func providerFilters(client *Route53APIFixtureStub, options ...func(awsProvider *AWSProvider)) *AWSProvider {
p := &AWSProvider{
clients: map[string]Route53API{defaultAWSProfile: client},
evaluateTargetHealth: false,
dryRun: false,
domainFilter: endpoint.NewDomainFilter([]string{}),
zoneIDFilter: provider.NewZoneIDFilter([]string{}),
zoneTypeFilter: provider.NewZoneTypeFilter(""),
zoneTagFilter: provider.NewZoneTagFilter([]string{}),
zonesCache: &zonesListCache{duration: 1 * time.Second},
}
for _, o := range options {
o(p)
}
return p
p := &AWSProvider{
clients: map[string]Route53API{defaultAWSProfile: client},
evaluateTargetHealth: false,
dryRun: false,
domainFilter: endpoint.NewDomainFilter([]string{}),
zoneIDFilter: provider.NewZoneIDFilter([]string{}),
zoneTypeFilter: provider.NewZoneTypeFilter(""),
zoneTagFilter: provider.NewZoneTagFilter([]string{}),
zonesCache: &zonesListCache{duration: 1 * time.Second},
}
for _, o := range options {
o(p)
}
return p
}

func WithDomainFilters(filters ...string) func(awsProvider *AWSProvider) {
return func(awsProvider *AWSProvider) {
awsProvider.domainFilter = endpoint.NewDomainFilter(filters)
}
return func(awsProvider *AWSProvider) {
awsProvider.domainFilter = endpoint.NewDomainFilter(filters)
}
}

func WithZoneIDFilters(filters ...string) func(awsProvider *AWSProvider) {
return func(awsProvider *AWSProvider) {
awsProvider.zoneIDFilter = provider.NewZoneIDFilter(filters)
}
return func(awsProvider *AWSProvider) {
awsProvider.zoneIDFilter = provider.NewZoneIDFilter(filters)
}
}

func WithZoneTagFilters(filters []string) func(awsProvider *AWSProvider) {
return func(awsProvider *AWSProvider) {
awsProvider.zoneTagFilter = provider.NewZoneTagFilter(filters)
}
return func(awsProvider *AWSProvider) {
awsProvider.zoneTagFilter = provider.NewZoneTagFilter(filters)
}
}

func NewRoute53APIFixtureStub(zones *HostedZones) *Route53APIFixtureStub {
route53Zones := make(map[string]*route53types.HostedZone)
zoneTags := make(map[string][]route53types.Tag)
for _, zone := range zones.Zones {
route53Zones[zone.ID] = &route53types.HostedZone{
Id: &zone.ID,
Name: &zone.Name,
}
zoneTags[cleanZoneID(zone.ID)] = zone.Tags
}
return &Route53APIFixtureStub{
zones: route53Zones,
zoneTags: zoneTags,
calls: make(map[string]int),
}
route53Zones := make(map[string]*route53types.HostedZone)
zoneTags := make(map[string][]route53types.Tag)
for _, zone := range zones.Zones {
route53Zones[zone.ID] = &route53types.HostedZone{
Id: &zone.ID,
Name: &zone.Name,
}
zoneTags[cleanZoneID(zone.ID)] = zone.Tags
}
return &Route53APIFixtureStub{
zones: route53Zones,
zoneTags: zoneTags,
calls: make(map[string]int),
}
}

func (r Route53APIFixtureStub) ListResourceRecordSets(ctx context.Context, input *route53.ListResourceRecordSetsInput, optFns ...func(options *route53.Options)) (*route53.ListResourceRecordSetsOutput, error) {
// TODO implement me
panic("implement me")
// TODO implement me
panic("implement me")
}

func (r Route53APIFixtureStub) ChangeResourceRecordSets(ctx context.Context, input *route53.ChangeResourceRecordSetsInput, optFns ...func(options *route53.Options)) (*route53.ChangeResourceRecordSetsOutput, error) {
// TODO implement me
panic("implement me")
// TODO implement me
panic("implement me")
}

func (r Route53APIFixtureStub) CreateHostedZone(ctx context.Context, input *route53.CreateHostedZoneInput, optFns ...func(*route53.Options)) (*route53.CreateHostedZoneOutput, error) {
// TODO implement me
panic("implement me")
// TODO implement me
panic("implement me")
}

func (r Route53APIFixtureStub) ListHostedZones(ctx context.Context, input *route53.ListHostedZonesInput, optFns ...func(options *route53.Options)) (*route53.ListHostedZonesOutput, error) {
r.calls["listhostedzones"]++
output := &route53.ListHostedZonesOutput{}
for _, zone := range r.zones {
output.HostedZones = append(output.HostedZones, *zone)
}
return output, nil
r.calls["listhostedzones"]++
output := &route53.ListHostedZonesOutput{}
for _, zone := range r.zones {
output.HostedZones = append(output.HostedZones, *zone)
}
return output, nil
}

func (r Route53APIFixtureStub) ListTagsForResource(ctx context.Context, input *route53.ListTagsForResourceInput, optFns ...func(options *route53.Options)) (*route53.ListTagsForResourceOutput, error) {
r.calls["listtagsforresource"]++
tags := r.zoneTags[*input.ResourceId]
return &route53.ListTagsForResourceOutput{
ResourceTagSet: &route53types.ResourceTagSet{
ResourceId: input.ResourceId,
ResourceType: input.ResourceType,
Tags: tags,
},
}, nil
r.calls["listtagsforresource"]++
tags := r.zoneTags[*input.ResourceId]
return &route53.ListTagsForResourceOutput{
ResourceTagSet: &route53types.ResourceTagSet{
ResourceId: input.ResourceId,
ResourceType: input.ResourceType,
Tags: tags,
},
}, nil
}

func unmarshalTestHelper(input string, obj any, t *testing.T) {
t.Helper()
path, _ := os.Getwd()
file, err := os.Open(path + input)
assert.NoError(t, err)
defer file.Close()
dec := yaml.NewDecoder(file)
err = dec.Decode(obj)
assert.NoError(t, err)
t.Helper()
path, _ := os.Getwd()
file, err := os.Open(path + input)
assert.NoError(t, err)
defer file.Close()
dec := yaml.NewDecoder(file)
err = dec.Decode(obj)
assert.NoError(t, err)
}
Loading