Update dependencies to use Go 1.21 & support VinylDNS 0.20.0 #94

brucedewald · 2024-04-09T03:38:04Z

Requirements

Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
All new code requires tests to ensure against regressions.

Description of the Change

I plan to submit a PR to terraform-provider-vinyldns in the near future to add some features, but before adding any functionality, I wanted to get this repo & terraform-provider-vinyldns in good shape. This PR should not be changing any functionality, just updating several things as described here:

Updating to use go 1.21 from 1.17 which was EOL'd in Aug 2022
Replacing deprecated go-aws-auth package with the official AWS SDK for AWS SigV4 signing of requests
Replacing deprecated ioutil package with io/os as appropriate
Updated Makefile to use latest version of VinylDNS for integration testing
Following changes to support version 0.20.0 of VinylDNS
- Updated Makefile to properly start/stop test containers
- Updating RecordSetChanges calls to set startfrom & nextID to be integers instead of strings
- Minor changes to tests to reflect current state of VinylDNS

Note: I'm not sure if I should be bumping the version of this package as part of this PR?

I will also be submitting a similar PR to terraform-provider-vinyldns in the very near future with similar updates.

Why Should This Be In The Package?

To ensure this package doesn't get further out of date.

Benefits

Support of current VinylDNS and using stable version of Go.

Possible Drawbacks

None to my knowledge.

Verification Process

Ran make all which does go test and then spins up docker container with VinylDNS 0.20.0 and does integration testing.

~ make all
test -z ""
go vet ./...
GO111MODULE=on go test ./... -cover
ok      github.com/vinyldns/go-vinyldns/vinyldns        0.375s  coverage: 84.6% of statements
GO111MODULE=on go build -ldflags "-X main.version=0.9.16" ./...
/Users/bdewald/go/1.21.9/src/github.com/vinyldns/vinyldns-0.20.0/quickstart/quickstart-vinyldns.sh \
                --clean
Killing running containers...
76382067785c
ea00febbe124
Removing containers...
76382067785c
ea00febbe124
Deleted Networks:
vinyldns_net

Clean up completed!
if [ ! -d "/Users/bdewald/go/1.21.9/src/github.com/vinyldns/vinyldns-0.20.0" ]; then \
                echo "github.com/vinyldns/vinyldns-0.20.0 not found in your GOPATH (necessary for acceptance tests), getting..."; \
                git clone \
                        --branch v0.20.0 \
                        https://github.com/vinyldns/vinyldns \
                        /Users/bdewald/go/1.21.9/src/github.com/vinyldns/vinyldns-0.20.0; \
        fi
/Users/bdewald/go/1.21.9/src/github.com/vinyldns/vinyldns-0.20.0/quickstart/quickstart-vinyldns.sh \
                --api \
                --version-tag 0.20.0
Starting VinylDNS (latest) the background...
[+] Running 2/0
[+] Running 2/5ldns_net                                                                                                                                       Created0.1s 
[+] Running 3/5ldns_net                                                                                                                                       Created0.2s 
[+] Running 4/5ldns_net                                                                                                                                       Created0.3s 
 ⠸ Network vinyldns_net                                                                                                                                       Created0.4s 
 ✔ Container vinyldns-api-integration                                                                                                                         Started0.2s 
 ! integration The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested        0.0s 
 ✔ Container vinyldns-api                                                                                                                                     Started0.3s 
 ! api The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested                0.0s 

Waiting for VinylDNS API at http://localhost:9000 ................OK

VinylDNS API started! You can connect to the API via http://localhost:9000
To clean up the running containers:
    /Users/bdewald/go/1.21.9/src/github.com/vinyldns/vinyldns-0.20.0/quickstart/quickstart-vinyldns.sh --clean
GO111MODULE=on go test ./... -tags=integration
ok      github.com/vinyldns/go-vinyldns/vinyldns        43.921s
cat vinyldns/version.go | grep 'var Version = "0.9.16"'
var Version = "0.9.16"
GO111MODULE=on go install ./...

Applicable Issues (Optional)

N/A

CLAassistant · 2024-04-09T03:38:10Z

All committers have signed the CLA.

Makefile

Aravindh-Raju

Some minor changes.

We will later have to change the buildQueryInt function to something like buildGlobalListQuery to include fqdn and recordType parameters that we now have for record's change history

vinyldns/endpoints.go

vinyldns/recordsets_resources.go

vinyldns/resources.go

Aravindh-Raju · 2024-04-17T11:08:39Z

@brucedewald, we will wait for other's review and decide if the above changes were necessary except for changing the startFrom type from any to int.

Aravindh-Raju

Looks good to me!

Makefile

Jay07GIT

LGTM.

brucedewald · 2024-04-18T17:49:39Z

Thank you @Aravindh-Raju @Jay07GIT for your reviews & approvals - what is needed to get this PR merged and a new release tag published?

brucedewald · 2024-05-06T22:50:10Z

@Aravindh-Raju @Jay07GIT - I wanted to follow up on my previous comment to see if you could provide any guidance here, thanks! Another PR is pending on a new release being created from this one: vinyldns/terraform-provider-vinyldns#116

Additionally, I plan to submit 1 or more feature PRs once this initial "clean up" work is done. Thank you!

nspadaccino

Looks good

arpit4ever

LGTM

nspadaccino

Hey @brucedewald, this repo requires all commits to be signed via GPG key, I see that your latest commit is signed, but the commits before that aren't, which is blocking us from merging. You can retroactively sign your unsigned commits using this command:

GIT_EDITOR=true git rebase --exec 'git commit --amend --no-edit -n -S' -i LAST_COMMIT_HASH_YOU_WANT_TO_SIGN

You can locally check if your commits are signed or not using
git log --show-signature

After you've rebased, you'll have to force-push the branch, since the commit history will have been edited.

brucedewald · 2024-06-10T20:10:08Z

Thanks @nspadaccino - All commits show as verified now

Jay07GIT suggested changes Apr 9, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

brucedewald mentioned this pull request Apr 17, 2024

Update dependencies to use Go 1.21 and use TF SDK v2 vinyldns/terraform-provider-vinyldns#116

Draft

Aravindh-Raju suggested changes Apr 17, 2024

View reviewed changes

vinyldns/endpoints.go Outdated Show resolved Hide resolved

vinyldns/endpoints.go Outdated Show resolved Hide resolved

vinyldns/recordsets_resources.go Outdated Show resolved Hide resolved

vinyldns/resources.go Outdated Show resolved Hide resolved

Aravindh-Raju approved these changes Apr 18, 2024

View reviewed changes

Jay07GIT suggested changes Apr 18, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Jay07GIT approved these changes Apr 18, 2024

View reviewed changes

nspadaccino added this to the 0.9.17 milestone Jun 10, 2024

nspadaccino approved these changes Jun 10, 2024

View reviewed changes

arpit4ever approved these changes Jun 10, 2024

View reviewed changes

nspadaccino self-requested a review June 10, 2024 19:48

nspadaccino requested changes Jun 10, 2024

View reviewed changes

brucedewald force-pushed the update_dependencies branch from 23e2305 to b01f6b4 Compare June 10, 2024 20:04

brucedewald and others added 6 commits June 10, 2024 13:09

update dependencies & tests

2f9e673

don't pin to a specific vinyldns version

1ac0f2f

change startfrom to int

44871a3

add ability to change vinyldns version for testing

cda4b5b

cleaning up recordsetchanges

fe4cc96

Update Makefile

8d24c82

brucedewald force-pushed the update_dependencies branch from b01f6b4 to 8d24c82 Compare June 10, 2024 20:09

nspadaccino approved these changes Jun 11, 2024

View reviewed changes

nspadaccino merged commit a38e17c into vinyldns:main Jun 11, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies to use Go 1.21 & support VinylDNS 0.20.0 #94

Update dependencies to use Go 1.21 & support VinylDNS 0.20.0 #94

brucedewald commented Apr 9, 2024 •

edited

Loading

CLAassistant commented Apr 9, 2024 •

edited

Loading

Aravindh-Raju left a comment

Aravindh-Raju commented Apr 17, 2024 •

edited

Loading

Aravindh-Raju left a comment

Jay07GIT left a comment

brucedewald commented Apr 18, 2024

brucedewald commented May 6, 2024

nspadaccino left a comment

arpit4ever left a comment

nspadaccino left a comment

brucedewald commented Jun 10, 2024

Update dependencies to use Go 1.21 & support VinylDNS 0.20.0 #94

Update dependencies to use Go 1.21 & support VinylDNS 0.20.0 #94

Conversation

brucedewald commented Apr 9, 2024 • edited Loading

Requirements

Description of the Change

Why Should This Be In The Package?

Benefits

Possible Drawbacks

Verification Process

Applicable Issues (Optional)

CLAassistant commented Apr 9, 2024 • edited Loading

Aravindh-Raju left a comment

Choose a reason for hiding this comment

Aravindh-Raju commented Apr 17, 2024 • edited Loading

Aravindh-Raju left a comment

Choose a reason for hiding this comment

Jay07GIT left a comment

Choose a reason for hiding this comment

brucedewald commented Apr 18, 2024

brucedewald commented May 6, 2024

nspadaccino left a comment

Choose a reason for hiding this comment

arpit4ever left a comment

Choose a reason for hiding this comment

nspadaccino left a comment

Choose a reason for hiding this comment

brucedewald commented Jun 10, 2024

brucedewald commented Apr 9, 2024 •

edited

Loading

CLAassistant commented Apr 9, 2024 •

edited

Loading

Aravindh-Raju commented Apr 17, 2024 •

edited

Loading