Skip to content

feat(dns): add record import command for BIND and JSON files - #5470

Open
jremy42 wants to merge 16 commits into
mainfrom
feat/dns-record-import
Open

feat(dns): add record import command for BIND and JSON files#5470
jremy42 wants to merge 16 commits into
mainfrom
feat/dns-record-import

Conversation

@jremy42

@jremy42 jremy42 commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Release note for CHANGELOG:

@jremy42
jremy42 requested review from a team and remyleone as code owners April 10, 2026 04:30
@github-actions github-actions Bot added the domain Domains issues, bugs and feature requests label Apr 10, 2026
@codecov-commenter

codecov-commenter commented Apr 10, 2026 •
edited
Loading

Copy link
Copy Markdown

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
2294 2 2292 17
View the top 2 failed test(s) by shortest run time 
github.com/scaleway/scaleway-cli/v2/core/human::TestMarshal
Stack Traces | 0s run time 
=== RUN   TestMarshal
--- FAIL: TestMarshal (0.00s)

github.com/scaleway/scaleway-cli/v2/core/human::TestMarshal/structWithMapsInSection
Stack Traces | 0s run time 
=== RUN   TestMarshal/structWithMapsInSection
    marshal_test.go:155: 
        	Error Trace:	.../core/human/marshal_test.go:91
        	Error:      	Not equal: 
        	            	expected: "String              This is a string\nInt                 42\nBool                true\nStrings.0           s1\nStrings.1           s2\nTime                35 years ago\nStruct.String       -\nStruct.Int          0\nStruct.Bool         false\nStruct.Time         a long while ago\nStruct.Stringer     a stringer\nStructs.0.String    Nested string\nStructs.0.Int       0\nStructs.0.Bool      false\nStructs.0.Time      a long while ago\nStructs.0.Stringer  a stringer\nStringer            a stringer\nStringerPtr         a stringer\nSize                13 kB\nBytes               AAE=\n\nMap String List:\nkey1  v1 v2\nkey2  v3 v4\n\nMap:\nkey1  v1\nkey2  v2"
        	            	actual  : "String              This is a string\nInt                 42\nBool                true\nStrings.0           s1\nStrings.1           s2\nTime                36 years ago\nStruct.String       -\nStruct.Int          0\nStruct.Bool         false\nStruct.Time         a long while ago\nStruct.Stringer     a stringer\nStructs.0.String    Nested string\nStructs.0.Int       0\nStructs.0.Bool      false\nStructs.0.Time      a long while ago\nStructs.0.Stringer  a stringer\nStringer            a stringer\nStringerPtr         a stringer\nSize                13 kB\nBytes               AAE=\n\nMap String List:\nkey1  v1 v2\nkey2  v3 v4\n\nMap:\nkey1  v1\nkey2  v2"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -5,3 +5,3 @@
        	            	 Strings.1           s2
        	            	-Time                35 years ago
        	            	+Time                36 years ago
        	            	 Struct.String       -
        	Test:       	TestMarshal/structWithMapsInSection
--- FAIL: TestMarshal/structWithMapsInSection (0.00s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@jremy42
jremy42 force-pushed the feat/dns-record-import branch from d8efdbe to ca1af47 Compare April 14, 2026 04:34
remyleone
remyleone requested changes Apr 17, 2026
View reviewed changes
Comment thread internal/namespaces/domain/v2beta1/custom_record_import.go Outdated
{
Name: "file",
Short: "Path to the zone file (bind) or JSON file",
Required: true,

@remyleone remyleone Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be helpful to add support for CanLoadFile

@jremy42 jremy42 Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread internal/namespaces/domain/v2beta1/custom_record_import.go Outdated
if err != nil {
return nil, fmt.Errorf("resolve file path: %w", err)
}
raw, err := os.ReadFile(abs)

@remyleone remyleone Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is already done by CanLoadFile

@jremy42 jremy42 Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@jremy42 jremy42 self-assigned this Jul 3, 2026
estellesoulard
estellesoulard reviewed Sep 17, 2026
View reviewed changes
Comment thread internal/namespaces/domain/v2beta1/record_import_parse.go
if err != nil {
return nil, err
}
if rec == nil {

@estellesoulard estellesoulard Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: in dnsRRToRecord(), if returned err is nil, it appears that rec is always populated (so this if could be removed)

estellesoulard
estellesoulard reviewed Sep 17, 2026
View reviewed changes
Comment thread internal/namespaces/domain/v2beta1/record_import_parse.go
}, nil
case *dns.SRV:
return &domain.Record{
Data: fmt.Sprintf("%d %d %d %s", v.Priority, v.Weight, v.Port, targetToData(v.Target)),

@estellesoulard estellesoulard Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not too familiar with tem, but why is Priority not set separately here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estellesoulard estellesoulard estellesoulard left review comments
@remyleone remyleone remyleone requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@jremy42 jremy42

Labels

domain Domains issues, bugs and feature requests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jremy42 @codecov-commenter @remyleone @estellesoulard