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

REFACTOR: Move intelligence from helpers.js to Go code #3273

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
66ca9c8
first try: A-records non-generated
tlimoncelli Dec 26, 2024
748ba39
m
tlimoncelli Dec 26, 2024
3f0080f
yamlv3
tlimoncelli Dec 27, 2024
cc14956
sort
tlimoncelli Dec 27, 2024
8dcd0ae
fmtjson and sort records
tlimoncelli Dec 27, 2024
2e753eb
dnscontrol fmt
tlimoncelli Dec 27, 2024
cffde5c
updates
tlimoncelli Dec 27, 2024
cb48444
updates
tlimoncelli Dec 27, 2024
84f3184
Merge branch 'tlim_tlim_sortparse' into tlim_rdata3
tlimoncelli Dec 28, 2024
ba11a9e
js_test sorts records
tlimoncelli Dec 28, 2024
433109c
sort 020-complexRequire.json 028-dextend.json
tlimoncelli Dec 28, 2024
830b749
fix sorting bug
tlimoncelli Dec 28, 2024
c5ee3e9
add README
tlimoncelli Dec 28, 2024
667f079
more
tlimoncelli Dec 28, 2024
97d7ab0
more
tlimoncelli Dec 28, 2024
0d5eea8
Merge branch 'tlim_tlim_sortparse' into tlim_rdata3
tlimoncelli Dec 28, 2024
2c1c7ce
Fix 028-dextend.json (remove duplicate record)
tlimoncelli Dec 28, 2024
9b778ae
fix D_EXTEND and ENSURE_ABSENT
tlimoncelli Dec 28, 2024
9fb5371
fix D_EXTEND and ENSURE_ABSENT
tlimoncelli Dec 28, 2024
1a3709c
before i rename CLOUDFLAREAPI_SINGLE_REDIRECT
tlimoncelli Dec 28, 2024
c8a74f3
all unit tests pass
tlimoncelli Dec 28, 2024
7e678d1
order
tlimoncelli Dec 28, 2024
7179a19
code complete
tlimoncelli Dec 28, 2024
055b5e6
remove debug statements
tlimoncelli Dec 28, 2024
7f8d90b
fix DefaultTTL()
tlimoncelli Dec 28, 2024
8db1c76
remove generators
tlimoncelli Dec 28, 2024
a36749f
remove DefaultTTL from 029-dextendsub.js
tlimoncelli Dec 28, 2024
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: 2 additions & 2 deletions commands/printIR.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/StackExchange/dnscontrol/v4/pkg/js"
"github.com/StackExchange/dnscontrol/v4/pkg/normalize"
"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
"github.com/StackExchange/dnscontrol/v4/pkg/rtypes"
"github.com/StackExchange/dnscontrol/v4/pkg/rtypectl"
"github.com/urfave/cli/v2"
)

Expand Down Expand Up @@ -130,7 +130,7 @@ func ExecuteDSL(args ExecuteDSLArgs) (*models.DNSConfig, error) {
return nil, fmt.Errorf("executing %s: %w", args.JSFile, err)
}

err = rtypes.PostProcess(dnsConfig.Domains)
err = rtypectl.TransformRawRecords(dnsConfig.Domains)
if err != nil {
return nil, err
}
Expand Down
23 changes: 10 additions & 13 deletions integrationTest/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/StackExchange/dnscontrol/v4/providers"
_ "github.com/StackExchange/dnscontrol/v4/providers/_all"
"github.com/StackExchange/dnscontrol/v4/providers/cloudflare"
"github.com/StackExchange/dnscontrol/v4/providers/cloudflare/rtypes/cfsingleredirect"
cfsingleredirect "github.com/StackExchange/dnscontrol/v4/providers/cloudflare/rtypes"
"github.com/miekg/dns/dnsutil"
)

Expand Down Expand Up @@ -61,7 +61,7 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[string
}

var metadata json.RawMessage
// CLOUDFLAREAPI tests related to CLOUDFLAREAPI_SINGLE_REDIRECT/CF_REDIRECT/CF_TEMP_REDIRECT
// CLOUDFLAREAPI tests related to CF_SINGLE_REDIRECT/CF_REDIRECT/CF_TEMP_REDIRECT
// requires metadata to enable this feature.
// In hindsight, I have no idea why this metadata flag is required to
// use this feature. Maybe because we didn't have the capabilities
Expand Down Expand Up @@ -521,12 +521,9 @@ func cfSingleRedirectEnabled() bool {
return ((*enableCFRedirectMode) != "")
}

func cfSingleRedirect(name string, code any, when, then string) *models.RecordConfig {
func cfSingleRedirect(name string, code uint16, when, then string) *models.RecordConfig {
r := makeRec("@", name, cfsingleredirect.SINGLEREDIRECT)
err := cfsingleredirect.FromRaw(r, []any{name, code, when, then})
if err != nil {
panic("Should not happen... cfSingleRedirect")
}
cfsingleredirect.MakeSingleRedirectFromRawRec(r, code, name, when, then)
return r
}

Expand Down Expand Up @@ -1965,14 +1962,14 @@ func makeTests() []*TestGroup {
tc("convert301", cfRedir("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1")),
),

testgroup("CLOUDFLAREAPI_SINGLE_REDIRECT",
testgroup("CF_SINGLE_REDIRECT",
only("CLOUDFLAREAPI"),
alltrue(cfSingleRedirectEnabled()),
tc("start301", cfSingleRedirect(`name1`, `301`, `http.host eq "cnn.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changecode", cfSingleRedirect(`name1`, `302`, `http.host eq "cnn.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changewhen", cfSingleRedirect(`name1`, `302`, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changethen", cfSingleRedirect(`name1`, `302`, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.msnbc.com", http.request.uri.path)`)),
tc("changename", cfSingleRedirect(`name1bis`, `302`, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.msnbc.com", http.request.uri.path)`)),
tc("start301", cfSingleRedirect(`name1`, 301, `http.host eq "cnn.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changecode", cfSingleRedirect(`name1`, 302, `http.host eq "cnn.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changewhen", cfSingleRedirect(`name1`, 302, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.cnn.com", http.request.uri.path)`)),
tc("changethen", cfSingleRedirect(`name1`, 302, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.msnbc.com", http.request.uri.path)`)),
tc("changename", cfSingleRedirect(`name1bis`, 302, `http.host eq "msnbc.slackoverflow.com"`, `concat("https://www.msnbc.com", http.request.uri.path)`)),
),

// CLOUDFLAREAPI: PROXY
Expand Down
3 changes: 2 additions & 1 deletion models/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type DomainConfig struct {

// Raw user-input from dnsconfig.js that will be processed into RecordConfigs later:
RawRecords []RawRecordConfig `json:"rawrecords,omitempty"`
DefaultTTL uint32 `json:"defaultTTL,omitempty"`

// Pending work to do for each provider. Provider may be a registrar or DSP.
pendingCorrectionsMutex sync.Mutex // Protect pendingCorrections*
Expand Down Expand Up @@ -131,7 +132,7 @@ func (dc *DomainConfig) Punycode() error {
return err
}
rec.SetTarget(t)
case "CLOUDFLAREAPI_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE":
case "CF_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE":
rec.SetTarget(rec.GetTargetField())
case "A", "AAAA", "CAA", "DHCID", "DNSKEY", "DS", "HTTPS", "LOC", "NAPTR", "SOA", "SSHFP", "SVCB", "TXT", "TLSA", "AZURE_ALIAS":
// Nothing to do.
Expand Down
12 changes: 8 additions & 4 deletions models/rawrecord.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ package models
// NOTE: Only newer rtypes are processed this way. Eventually the
// legacy types will be converted.
type RawRecordConfig struct {
Type string `json:"type"`
Args []any `json:"args,omitempty"`
Metas []map[string]any `json:"metas,omitempty"`
TTL uint32 `json:"ttl,omitempty"`
Type string `json:"type"`
Args []any `json:"args,omitempty"`
Metas []map[string]any `json:"metas,omitempty"`
TTL uint32 `json:"ttl,omitempty"`
SubDomain string `json:"subdomain,omitempty"`

// Override NO_PURGE and delete this record
EnsureAbsent bool `json:"ensure_absent,omitempty"`
}
11 changes: 7 additions & 4 deletions models/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (
// CF_REDIRECT
// CF_TEMP_REDIRECT
// CF_WORKER_ROUTE
// CLOUDFLAREAPI_SINGLE_REDIRECT
// CF_SINGLE_REDIRECT
// CLOUDNS_WR
// FRAME
// IMPORT_TRANSFORM
Expand All @@ -53,7 +53,7 @@ import (
// URL301
// WORKER_ROUTE
//
// NOTE: All NEW record types should be prefixed with the provider name (Correct: CLOUDFLAREAPI_SINGLE_REDIRECT. Wrong: CF_REDIRECT)
// NOTE: All NEW record types should be prefixed with the provider name (Correct: CF_SINGLE_REDIRECT. Wrong: CF_REDIRECT)
//
// Notes about the fields:
//
Expand Down Expand Up @@ -99,6 +99,9 @@ type RecordConfig struct {
Metadata map[string]string `json:"meta,omitempty"`
Original interface{} `json:"-"` // Store pointer to provider-specific record object. Used in diffing.

Comparable string `json:"-"`
Display string `json:"-"`

// If you add a field to this struct, also add it to the list in the UnmarshalJSON function.
MxPreference uint16 `json:"mxpreference,omitempty"`
SrvPriority uint16 `json:"srvpriority,omitempty"`
Expand Down Expand Up @@ -615,7 +618,7 @@ func Downcase(recs []*RecordConfig) {
// Target is case insensitive. Downcase it.
r.target = strings.ToLower(r.target)
// BUGFIX(tlim): isn't ALIAS in the wrong case statement?
case "A", "CAA", "CLOUDFLAREAPI_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "DHCID", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TXT":
case "A", "CAA", "CF_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "DHCID", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TXT":
// Do nothing. (IP address or case sensitive target)
case "SOA":
if r.target != "DEFAULT_NOT_SET." {
Expand All @@ -639,7 +642,7 @@ func CanonicalizeTargets(recs []*RecordConfig, origin string) {
case "ALIAS", "ANAME", "CNAME", "DNAME", "DS", "DNSKEY", "MX", "NS", "NAPTR", "PTR", "SRV":
// Target is a hostname that might be a shortname. Turn it into a FQDN.
r.target = dnsutil.AddOrigin(r.target, originFQDN)
case "A", "AKAMAICDN", "CAA", "DHCID", "CLOUDFLAREAPI_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "HTTPS", "IMPORT_TRANSFORM", "LOC", "SSHFP", "SVCB", "TLSA", "TXT":
case "A", "AKAMAICDN", "CAA", "DHCID", "CF_SINGLE_REDIRECT", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "HTTPS", "IMPORT_TRANSFORM", "LOC", "SSHFP", "SVCB", "TLSA", "TXT":
// Do nothing.
case "SOA":
if r.target != "DEFAULT_NOT_SET." {
Expand Down
19 changes: 10 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/diff2/handsoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package diff2
// NO_PURGE, ENSURE_ABSENT and IGNORE*() features.

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -138,7 +139,7 @@ func handsoff(
msgs = append(msgs, fmt.Sprintf(" %s %s %s", r.GetLabelFQDN(), r.Type, r.GetTargetCombined()))
}
if !unmanagedSafely {
return nil, nil, fmt.Errorf(strings.Join(msgs, "\n") +
return nil, nil, errors.New(strings.Join(msgs, "\n") +
"\nERROR: Unsafe to continue. Add DISABLE_IGNORE_SAFETY_CHECK to D() to override")
}
}
Expand Down
42 changes: 42 additions & 0 deletions pkg/js/README-parse_tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

# Parse Tests

The `parse_tests` directory contains test cases for `js_test.go`. `js_test.go`
scans for files named `DDD-*.js` where `DDD` is a three-digit number.

* `parse_tests/001-basic.js` -- The dnsconfig.js file.
* `parse_tests/001-basic.json` -- The EXPECTED output of "print-ir" for the `.js` file.
* `parse_tests/001-basic.json.ACTUAL` -- The ACTUAL output of "print-ir" for the `.js` file (not saved in git)
* `parse_tests/001-basic/foo.com.zone` -- Zonefiles from the domains mentioned in dnsconfig.js

NOTE: The zonefiles are only tested if a matching `DDD-name/DOMAINNAME.zone` file exists.

Any files committed to Git should be in standard format.

# Fix formatting

Fix the `.js` formatting:

```
cd parse_tests
for i in *.js ; do echo ========== $i ; dnscontrol fmt -i $i -o $i ; done
```

Fix the `.json` formatting:

```
cd parse_tests
fmtjson *.json *.json.ACTUAL
```

# Copy actuals to expected.

Back-port the ACTUAL results to the expected results:

(This is dangerous. You may be committing buggy results to the "expected" files. Carefully inspect the resulting PR.)

```
cd parse_tests
fmtjson *.json *.json.ACTUAL
for i in *.ACTUAL ; do f=$(basename $i .ACTUAL) ; cp $i $f ; done
```
2 changes: 2 additions & 0 deletions pkg/js/helpers-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var A = rawrecordBuilder('A');
var CF_SINGLE_REDIRECT = rawrecordBuilder('CF_SINGLE_REDIRECT');
22 changes: 13 additions & 9 deletions pkg/js/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ var conf = {

var defaultArgs = [];

var global_currentExtendSubDomain = ''; // The current D_EXTEND() zone (minus the parent zonename).

function initialize() {
conf = {
registrars: [],
Expand Down Expand Up @@ -140,6 +142,7 @@ function processDargs(m, domain) {

// D(name,registrar): Create a DNS Domain. Use the parameters as records and mods.
function D(name, registrar) {
global_currentExtendSubDomain = '';
var domain = newDomain(name, registrar);
for (var i = 0; i < defaultArgs.length; i++) {
processDargs(defaultArgs[i], domain);
Expand All @@ -165,6 +168,7 @@ function INCLUDE(name) {
}
return function (d) {
d.records.push.apply(d.records, domain.obj.records);
d.rawrecords.push.apply(d.rawrecords, domain.obj.rawrecords);
};
}

Expand All @@ -177,10 +181,14 @@ function D_EXTEND(name) {
' was not declared yet and therefore cannot be updated. Use D() before.'
);
}
domain.obj.subdomain = name.substr(
global_currentExtendSubDomain = name.substr(
0,
name.length - domain.obj.name.length - 1
);
domain.obj.subdomain = global_currentExtendSubDomain;
// global_currentExtendSubDomain is used by RawRecords. (new)
// domain.obj.subdomain is used by recordBuilder(). (legacy)

for (var i = 1; i < arguments.length; i++) {
var m = arguments[i];
processDargs(m, domain.obj);
Expand Down Expand Up @@ -280,9 +288,6 @@ function DnsProvider(name, nsCount) {
};
}

// A(name,ip, recordModifiers...)
var A = recordBuilder('A');

// AAAA(name,ip, recordModifiers...)
var AAAA = recordBuilder('AAAA');

Expand Down Expand Up @@ -2065,6 +2070,7 @@ function rawrecordBuilder(type) {
return function (d) {
var record = {
type: type,
subdomain: global_currentExtendSubDomain,
};

// Process the args: Functions are executed, objects are assumed to
Expand All @@ -2082,6 +2088,9 @@ function rawrecordBuilder(type) {
if (_.isFunction(r)) {
r(record);
} else if (_.isObject(r)) {
if (r.transform && _.isArray(r.transform)) {
r.transform = format_tt(r.transform);
}
processedMetas.push(r);
} else {
processedArgs.push(r);
Expand All @@ -2098,8 +2107,3 @@ function rawrecordBuilder(type) {
};
};
}

// PLEASE KEEP THIS LIST ALPHABETICAL!

// CLOUDFLAREAPI:
var CF_SINGLE_REDIRECT = rawrecordBuilder('CLOUDFLAREAPI_SINGLE_REDIRECT');
22 changes: 19 additions & 3 deletions pkg/js/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
"github.com/StackExchange/dnscontrol/v4/models"
"github.com/StackExchange/dnscontrol/v4/pkg/printer"
"github.com/StackExchange/dnscontrol/v4/pkg/rfc4183"
"github.com/StackExchange/dnscontrol/v4/pkg/rtypectl"
"github.com/StackExchange/dnscontrol/v4/pkg/transform"
_ "github.com/StackExchange/dnscontrol/v4/rtypes"
"github.com/robertkrimen/otto" // load underscore js into vm by default
_ "github.com/robertkrimen/otto/underscore" // required by otto
"github.com/xddxdd/ottoext/fetch"
Expand All @@ -25,6 +27,10 @@ import (
var helpersJsStatic string
var helpersJsFileName = "pkg/js/helpers.js"

//go:embed helpers-types.js
var helpersTypesJsStatic string
var helpersTypesJsFileName = "pkg/js/helpers-types.js"

// currentDirectory is the current directory as used by require().
// This is used to emulate nodejs-style require() directory handling.
// If require("a/b/c.js") is called, any require() statement in c.js
Expand Down Expand Up @@ -110,22 +116,32 @@ func ExecuteJavascriptString(script []byte, devMode bool, variables map[string]s
if err = json.Unmarshal([]byte(str), conf); err != nil {
return nil, err
}

err = rtypectl.TransformRawRecords(conf.Domains)
if err != nil {
return nil, err
}

return conf, nil
}

// GetHelpers returns the contents of helpers.js, or the embedded version.
func GetHelpers(devMode bool) string {
if devMode {
// Load the file:
b, err := os.ReadFile(helpersJsFileName)
a, err := os.ReadFile(helpersJsFileName)
if err != nil {
log.Fatal(err)
}
b, err := os.ReadFile(helpersTypesJsFileName)
if err != nil {
log.Fatal(err)
}
return string(b)
return string(a) + "\n" + string(b)
}

// Return the embedded bytes:
return helpersJsStatic
return helpersJsStatic + "\n" + helpersTypesJsStatic
}

func require(call otto.FunctionCall) otto.Value {
Expand Down
Loading
Loading