Skip to content

Commit 8fd0f0f

Browse files
marceljkmarceljk
committed
fix: review feedback
- create new function which formats the dns records - add test cases - moved the formatting of txt records - in create command to parseInput - in update to separate function
1 parent 602c42f commit 8fd0f0f

File tree

6 files changed

+286
-106
lines changed

6 files changed

+286
-106
lines changed

‎internal/cmd/dns/record-set/create/create.go‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"math"
87

98
"github.com/goccy/go-yaml"
9+
"github.com/spf13/cobra"
1010
"github.com/stackitcloud/stackit-cli/internal/pkg/args"
1111
"github.com/stackitcloud/stackit-cli/internal/pkg/errors"
1212
"github.com/stackitcloud/stackit-cli/internal/pkg/examples"
@@ -17,8 +17,6 @@ import (
1717
dnsUtils "github.com/stackitcloud/stackit-cli/internal/pkg/services/dns/utils"
1818
"github.com/stackitcloud/stackit-cli/internal/pkg/spinner"
1919
"github.com/stackitcloud/stackit-cli/internal/pkg/utils"
20-
21-
"github.com/spf13/cobra"
2220
"github.com/stackitcloud/stackit-sdk-go/services/dns"
2321
"github.com/stackitcloud/stackit-sdk-go/services/dns/wait"
2422
)
@@ -139,6 +137,20 @@ func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) {
139137
Type: flags.FlagWithDefaultToStringValue(p, cmd, typeFlag),
140138
}
141139

140+
if model.Type == txtType {
141+
for idx := range model.Records {
142+
// Based on RFC 1035 section 2.3.4, TXT Records are limited to 255 Characters
143+
// Longer strings need to be split into multiple records
144+
if len(model.Records[idx]) > 255 {
145+
var err error
146+
model.Records[idx], err = dnsUtils.FormatTxtRecord(model.Records[idx])
147+
if err != nil {
148+
return nil, err
149+
}
150+
}
151+
}
152+
}
153+
142154
if p.IsVerbosityDebug() {
143155
modelStr, err := print.BuildDebugStrFromInputModel(model)
144156
if err != nil {
@@ -154,23 +166,7 @@ func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) {
154166
func buildRequest(ctx context.Context, model *inputModel, apiClient *dns.APIClient) dns.ApiCreateRecordSetRequest {
155167
records := make([]dns.RecordPayload, 0)
156168
for _, r := range model.Records {
157-
result := r
158-
if len(r) > 255 && model.Type == txtType {
159-
result = ""
160-
length := float64(len(r))
161-
chunks := int(math.Ceil(length / 255))
162-
for i := range chunks {
163-
skip := 255 * i
164-
if i == chunks-1 {
165-
// Append the left record content
166-
result += fmt.Sprintf("%q", r[0+skip:])
167-
} else {
168-
// Add 255 characters of the record data quoted to the result
169-
result += fmt.Sprintf("%q ", r[0+skip:255+skip])
170-
}
171-
}
172-
}
173-
records = append(records, dns.RecordPayload{Content: utils.Ptr(result)})
169+
records = append(records, dns.RecordPayload{Content: utils.Ptr(r)})
174170
}
175171

176172
req := apiClient.CreateRecordSet(ctx, model.ProjectId, model.ZoneId)

‎internal/cmd/dns/record-set/create/create_test.go‎

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func fixtureRequest(mods ...func(request *dns.ApiCreateRecordSetRequest)) dns.Ap
8484
}
8585

8686
func TestParseInput(t *testing.T) {
87-
tests := []struct {
87+
var tests = []struct {
8888
description string
8989
flagValues map[string]string
9090
recordFlagValues []string
@@ -244,8 +244,27 @@ func TestParseInput(t *testing.T) {
244244
model.Records = append(model.Records, "1.2.3.4", "5.6.7.8")
245245
}),
246246
},
247-
}
247+
{
248+
description: "TXT record with > 255 characters",
249+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
250+
flagValues[typeFlag] = txtType
251+
flagValues[recordFlag] = strings.Join(recordTxtOver255Char, "")
252+
}),
253+
isValid: true,
254+
expectedModel: fixtureInputModel(func(model *inputModel) {
255+
var content string
256+
for idx, val := range recordTxtOver255Char {
257+
content += fmt.Sprintf("%q", val)
258+
if idx != len(recordTxtOver255Char)-1 {
259+
content += " "
260+
}
261+
}
248262

263+
model.Records = []string{content}
264+
model.Type = txtType
265+
}),
266+
},
267+
}
249268
for _, tt := range tests {
250269
t.Run(tt.description, func(t *testing.T) {
251270
p := print.NewPrinter()
@@ -334,35 +353,6 @@ func TestBuildRequest(t *testing.T) {
334353
Type: utils.Ptr(defaultType),
335354
}),
336355
},
337-
{
338-
description: "add TXT record with > 255 characters",
339-
model: fixtureInputModel(func(model *inputModel) {
340-
model.Type = txtType
341-
model.Records = []string{strings.Join(recordTxtOver255Char, "")}
342-
}),
343-
expectedRequest: func() dns.ApiCreateRecordSetRequest {
344-
var content string
345-
for idx, val := range recordTxtOver255Char {
346-
content += fmt.Sprintf("%q", val)
347-
if idx != len(recordTxtOver255Char)-1 {
348-
content += " "
349-
}
350-
}
351-
352-
return testClient.CreateRecordSet(testCtx, testProjectId, testZoneId).
353-
CreateRecordSetPayload(dns.CreateRecordSetPayload{
354-
Name: utils.Ptr("example.com"),
355-
Records: &[]dns.RecordPayload{
356-
{
357-
Content: utils.Ptr(content),
358-
},
359-
},
360-
Type: utils.Ptr(txtType),
361-
Comment: utils.Ptr("comment"),
362-
Ttl: utils.Ptr(int64(3600)),
363-
})
364-
}(),
365-
},
366356
}
367357

368358
for _, tt := range tests {

‎internal/cmd/dns/record-set/update/update.go‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package update
33
import (
44
"context"
55
"fmt"
6-
"math"
76

87
"github.com/stackitcloud/stackit-cli/internal/pkg/args"
98
"github.com/stackitcloud/stackit-cli/internal/pkg/errors"
@@ -85,6 +84,13 @@ func NewCmd(p *print.Printer) *cobra.Command {
8584
}
8685
model.Type = typeLabel
8786

87+
if utils.PtrString(model.Type) == txtType {
88+
err = parseTxtRecord(model.Records)
89+
if err != nil {
90+
return err
91+
}
92+
}
93+
8894
if !model.AssumeYes {
8995
prompt := fmt.Sprintf("Are you sure you want to update record set %s of zone %s?", recordSetLabel, zoneLabel)
9096
err = p.PromptForConfirmation(prompt)
@@ -174,28 +180,33 @@ func parseInput(p *print.Printer, cmd *cobra.Command, inputArgs []string) (*inpu
174180
return &model, nil
175181
}
176182

183+
func parseTxtRecord(records *[]string) error {
184+
if records == nil {
185+
return nil
186+
}
187+
if len(*records) == 0 {
188+
return nil
189+
}
190+
191+
for idx := range *records {
192+
var err error
193+
// Based on RFC 1035 section 2.3.4, TXT Records are limited to 255 Characters.
194+
// Longer strings need to be split into multiple records
195+
(*records)[idx], err = dnsUtils.FormatTxtRecord((*records)[idx])
196+
if err != nil {
197+
return err
198+
}
199+
}
200+
201+
return nil
202+
}
203+
177204
func buildRequest(ctx context.Context, model *inputModel, apiClient *dns.APIClient) dns.ApiPartialUpdateRecordSetRequest {
178205
var records *[]dns.RecordPayload = nil
179206
if model.Records != nil {
180207
records = utils.Ptr(make([]dns.RecordPayload, 0))
181208
for _, r := range *model.Records {
182-
result := r
183-
if len(r) > 255 && utils.PtrString(model.Type) == txtType {
184-
result = ""
185-
length := float64(len(r))
186-
chunks := int(math.Ceil(length / 255))
187-
for i := range chunks {
188-
skip := 255 * i
189-
if i == chunks-1 {
190-
// Append the left record content
191-
result += fmt.Sprintf("%q", r[0+skip:])
192-
} else {
193-
// Add 255 characters of the record data quoted to the result
194-
result += fmt.Sprintf("%q ", r[0+skip:255+skip])
195-
}
196-
}
197-
}
198-
records = utils.Ptr(append(*records, dns.RecordPayload{Content: utils.Ptr(result)}))
209+
records = utils.Ptr(append(*records, dns.RecordPayload{Content: utils.Ptr(r)}))
199210
}
200211
}
201212

‎internal/cmd/dns/record-set/update/update_test.go‎

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package update
22

33
import (
44
"context"
5-
"fmt"
6-
"strings"
75
"testing"
86

97
"github.com/stackitcloud/stackit-cli/internal/pkg/globalflags"
@@ -26,11 +24,12 @@ var testProjectId = uuid.NewString()
2624
var testZoneId = uuid.NewString()
2725
var testRecordSetId = uuid.NewString()
2826

29-
var recordTxtOver255Char = []string{
30-
"foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoo",
31-
"foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoo",
32-
"foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobar",
33-
}
27+
var (
28+
text255Characters = "foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoo"
29+
text256Characters = "foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoob"
30+
result256Characters = "\"foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoo\" \"b\""
31+
text4050Characters = "foobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoofoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoobarfoo"
32+
)
3433

3534
func fixtureArgValues(mods ...func(argValues []string)) []string {
3635
argValues := []string{
@@ -315,6 +314,71 @@ func TestParseInput(t *testing.T) {
315314
}
316315
}
317316

317+
func TestParseTxtRecord(t *testing.T) {
318+
tests := []struct {
319+
description string
320+
records *[]string
321+
expectedResult *[]string
322+
isValid bool
323+
shouldErr bool
324+
}{
325+
{
326+
description: "empty",
327+
records: nil,
328+
expectedResult: nil,
329+
isValid: true,
330+
},
331+
{
332+
description: "base",
333+
records: &[]string{"foobar"},
334+
expectedResult: &[]string{"foobar"},
335+
isValid: true,
336+
},
337+
{
338+
description: "input has length of 255 characters and should not split",
339+
records: &[]string{text255Characters},
340+
expectedResult: &[]string{text255Characters},
341+
isValid: true,
342+
},
343+
{
344+
description: "input has length 256 characters and should split",
345+
records: &[]string{text256Characters},
346+
expectedResult: &[]string{result256Characters},
347+
isValid: true,
348+
},
349+
{
350+
description: "input has length 4050 characters and should fail",
351+
records: &[]string{text4050Characters},
352+
isValid: false,
353+
},
354+
}
355+
for _, tt := range tests {
356+
t.Run(tt.description, func(t *testing.T) {
357+
err := parseTxtRecord(tt.records)
358+
if err != nil {
359+
if !tt.isValid {
360+
return
361+
}
362+
t.Fatalf("should not fail but got error: %v", err)
363+
return
364+
}
365+
if err == nil && !tt.isValid {
366+
t.Fatalf("should fail but got none")
367+
return
368+
}
369+
370+
if !tt.isValid {
371+
t.Fatalf("should fail but got none")
372+
return
373+
}
374+
diff := cmp.Diff(tt.expectedResult, tt.records)
375+
if diff != "" {
376+
t.Fatalf("Data does not match: %s", diff)
377+
}
378+
})
379+
}
380+
}
381+
318382
func TestBuildRequest(t *testing.T) {
319383
tests := []struct {
320384
description string
@@ -339,34 +403,6 @@ func TestBuildRequest(t *testing.T) {
339403
expectedRequest: testClient.PartialUpdateRecordSet(testCtx, testProjectId, testZoneId, testRecordSetId).
340404
PartialUpdateRecordSetPayload(dns.PartialUpdateRecordSetPayload{}),
341405
},
342-
{
343-
description: "update TXT record with > 255 characters",
344-
model: fixtureInputModel(func(model *inputModel) {
345-
model.Type = utils.Ptr(txtType)
346-
model.Records = utils.Ptr([]string{strings.Join(recordTxtOver255Char, "")})
347-
}),
348-
expectedRequest: func() dns.ApiPartialUpdateRecordSetRequest {
349-
var content string
350-
for idx, val := range recordTxtOver255Char {
351-
content += fmt.Sprintf("%q", val)
352-
if idx != len(recordTxtOver255Char)-1 {
353-
content += " "
354-
}
355-
}
356-
357-
return testClient.PartialUpdateRecordSet(testCtx, testProjectId, testZoneId, testRecordSetId).
358-
PartialUpdateRecordSetPayload(dns.PartialUpdateRecordSetPayload{
359-
Name: utils.Ptr("example.com"),
360-
Records: &[]dns.RecordPayload{
361-
{
362-
Content: utils.Ptr(content),
363-
},
364-
},
365-
Comment: utils.Ptr("comment"),
366-
Ttl: utils.Ptr(int64(3600)),
367-
})
368-
}(),
369-
},
370406
}
371407

372408
for _, tt := range tests {

‎internal/pkg/services/dns/utils/utils.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package utils
33
import (
44
"context"
55
"fmt"
6+
"math"
67

78
"github.com/stackitcloud/stackit-cli/internal/pkg/utils"
89
"github.com/stackitcloud/stackit-sdk-go/services/dns"
@@ -36,3 +37,29 @@ func GetRecordSetType(ctx context.Context, apiClient DNSClient, projectId, zoneI
3637
}
3738
return resp.Rrset.Type, nil
3839
}
40+
41+
func FormatTxtRecord(input string) (string, error) {
42+
length := float64(len(input))
43+
if length <= 255 {
44+
return input, nil
45+
}
46+
// Max length with quotes and white spaces is 4096. Without the quotes and white spaces the max length is 4049
47+
if length > 4049 {
48+
return "", fmt.Errorf("max input length is 4049. The length of the input is %v", length)
49+
}
50+
51+
result := ""
52+
chunks := int(math.Ceil(length / 255))
53+
for i := range chunks {
54+
skip := 255 * i
55+
if i == chunks-1 {
56+
// Append the left record content
57+
result += fmt.Sprintf("%q", input[0+skip:])
58+
} else {
59+
// Add 255 characters of the record data quoted to the result
60+
result += fmt.Sprintf("%q ", input[0+skip:255+skip])
61+
}
62+
}
63+
64+
return result, nil
65+
}

0 commit comments

Comments
 (0)