k-v.io: on DELETE, don't return the deleted value

We don't return the deleted value because doing that would have the
unintended consequence of postponing the deletion: downstream caching
servers would cache the deleted value for up to three more minutes. We'd
rather have the key deleted sooner rather than later.

Some APIs, e.g. etcd's, return a list of deleted values on return: those
APIs can afford to do so because they don't need to worry about DNS
propagation.

We also lengthen the timeout of an `etcd` API call from 500 msec to 1928
msecs; 500 msec was too close; some calls routinely took 480 msec to
complete, and we wanted more headroom.

We also no longer do two `etcd` operations when we delete a value.
Previously we would do a GET followed by a DELETE, but since we're not
returning the value deleted, there's no point to the GET. Furthermore,
the GET was never necessary, for the `etcd` DELETE API call returned the
values deleted.

Drive-by:
- README: install gingko the proper way, with `go install`

[fixes #17]
This commit is contained in:
Brian Cunnie
2022-04-11 09:42:54 -07:00
parent 4d6b4375a3
commit 033cf481d7
4 changed files with 32 additions and 27 deletions

View File

@@ -31,7 +31,7 @@ dig @localhost 192.168.0.1.sslip.io +short
## Quick Start Tests
```bash
go get github.com/onsi/ginkgo/v2/ginkgo
go install github.com/onsi/ginkgo/v2/ginkgo@latest
go get github.com/onsi/gomega/...
sudo ~/go/bin/ginkgo -r .
# sudo is required on Linux, but not on macOS, to bind to privileged port 53

View File

@@ -147,8 +147,8 @@ var _ = Describe("sslip.io-dns-server", func() {
`TypeTXT my-key.k-v.io. \? \["MyValue"\]`),
Entry(`deleting a value: TXT for delete.my-key.k-v.io"`,
"@127.0.0.1 delete.my-key.k-v.io txt +short",
`"MyValue"`,
`TypeTXT delete.my-key.k-v.io. \? \["MyValue"\]`),
`\A\z`,
`TypeTXT delete.my-key.k-v.io. \? nil, SOA delete.my-key.k-v.io. briancunnie.gmail.com. 2022020800 900 900 1800 180\n$`),
Entry(`getting a non-existent value: TXT for my-key.k-v.io"`,
"@127.0.0.1 my-key.k-v.io txt +short",
`\A\z`,
@@ -256,14 +256,21 @@ var _ = Describe("sslip.io-dns-server", func() {
Eventually(digSession, 1).Should(Exit(0))
Eventually(string(serverSession.Err.Contents())).Should(MatchRegexp(`TypeTXT b.k-v.io. \? \["a"\]`))
})
It(`the DELETE has a three-minute TTL`, func() {
It(`the DELETE returns no records so that value cached in downstream DNS servers expires more quickly`, func() {
digArgs = "@localhost delete.b.k-v.io txt -p " + strconv.Itoa(port)
digCmd = exec.Command("dig", strings.Split(digArgs, " ")...)
digSession, err = Start(digCmd, GinkgoWriter, GinkgoWriter)
Expect(err).ToNot(HaveOccurred())
Eventually(digSession).Should(Say(`delete.b.k-v.io. 180 IN TXT "a"`))
Eventually(digSession, 1).Should(Exit(0))
Eventually(string(serverSession.Err.Contents())).Should(MatchRegexp(`TypeTXT delete.b.k-v.io. \? \["a"\]`))
Eventually(string(serverSession.Err.Contents())).Should(MatchRegexp(`TypeTXT delete.b.k-v.io. \? nil, SOA delete.b.k-v.io. briancunnie.gmail.com. 2022020800 900 900 1800 180`))
})
It(`the DELETE on a non-existent key behaves the same as the DELETE on an existing key`, func() {
digArgs = "@localhost delete.b.k-v.io txt -p " + strconv.Itoa(port)
digCmd = exec.Command("dig", strings.Split(digArgs, " ")...)
digSession, err = Start(digCmd, GinkgoWriter, GinkgoWriter)
Expect(err).ToNot(HaveOccurred())
Eventually(digSession, 1).Should(Exit(0))
Eventually(string(serverSession.Err.Contents())).Should(MatchRegexp(`TypeTXT delete.b.k-v.io. \? nil, SOA delete.b.k-v.io. briancunnie.gmail.com. 2022020800 900 900 1800 180`))
})
})
When(`a record for an "_acme-challenge" domain is queried`, func() {

View File

@@ -88,7 +88,6 @@ type KvCustomizations map[string][]dnsmessage.TXTResource
// There's nothing like global variables to make my heart pound with joy.
// Some of these are global because they are, in essence, constants which
// I don't want to waste time recreating with every function call.
// But `Customizations` is a true global variable.
var (
ipv4REDots = regexp.MustCompile(`(^|[.-])(((25[0-5]|(2[0-4]|1?[0-9])?[0-9])\.){3}(25[0-5]|(2[0-4]|1?[0-9])?[0-9]))($|[.-])`)
ipv4REDashes = regexp.MustCompile(`(^|[.-])(((25[0-5]|(2[0-4]|1?[0-9])?[0-9])-){3}(25[0-5]|(2[0-4]|1?[0-9])?[0-9]))($|[.-])`)
@@ -116,7 +115,15 @@ var (
VersionDate = "0001/01/01-99:99:99-0800"
VersionGitHash = "cafexxx"
MetricsBufferSize = 100
MetricsBufferSize = 100 // big enough to run our tests, and small enough to prevent DNS amplification attacks
// etcdContextTimeout — the duration (context) that we wait for etcd to get back to us
// - etcd queries on the nameserver take as long as 482 milliseconds on the "slow" server, 247 on the "fast"
// - round-trip time from my house in San Francisco to ns-azure in Singapore is 190 milliseconds
// - time between queries with `dig` on my macOS Monterey is 5000 milliseconds, three queries before giving up
// - quadruple the headroom for queries 4 x 482 = 1928, should still leave enough room to get answer back
// within the 5000 milliseconds
etcdContextTimeout = 1928 * time.Millisecond
TxtKvCustomizations = KvCustomizations{}
Customizations = DomainCustomizations{
@@ -652,6 +659,7 @@ func NameToA(fqdnString string) []dnsmessage.AResource {
ipv4address := net.ParseIP(match).To4()
// We shouldn't reach here because `match` should always be valid, but we're not optimists
if ipv4address == nil {
// e.g. "ubuntu20.04.235.249.181-notify.sslip.io."
log.Printf("----> Should be valid A but isn't: %s\n", fqdn) // TODO: delete this
return []dnsmessage.AResource{}
}
@@ -864,7 +872,7 @@ func (x *Xip) getKv(key string) ([]dnsmessage.TXTResource, error) {
}
return nil, nil
}
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
ctx, cancel := context.WithTimeout(context.Background(), etcdContextTimeout)
defer cancel()
resp, err := x.Etcd.Get(ctx, key)
if err != nil {
@@ -888,7 +896,7 @@ func (x *Xip) putKv(key, value string) ([]dnsmessage.TXTResource, error) {
}
return TxtKvCustomizations[key], nil
}
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
ctx, cancel := context.WithTimeout(context.Background(), etcdContextTimeout)
defer cancel()
_, err := x.Etcd.Put(ctx, key, value)
if err != nil {
@@ -899,28 +907,18 @@ func (x *Xip) putKv(key, value string) ([]dnsmessage.TXTResource, error) {
func (x *Xip) deleteKv(key string) ([]dnsmessage.TXTResource, error) {
if x.isEtcdNil() {
if deletedKey, ok := TxtKvCustomizations[key]; ok {
if _, ok := TxtKvCustomizations[key]; ok {
delete(TxtKvCustomizations, key)
return deletedKey, nil
}
return nil, nil
}
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
ctx, cancel := context.WithTimeout(context.Background(), etcdContextTimeout)
defer cancel()
getResp, err := x.Etcd.Get(ctx, key) // is the key set?
_, err := x.Etcd.Delete(ctx, key)
if err != nil {
return nil, fmt.Errorf(`couldn't GET "%s": %w`, key, err)
return nil, fmt.Errorf("couldn't DELETE (key %s): %w", key, err)
}
if len(getResp.Kvs) == 0 { // nothing to delete
return []dnsmessage.TXTResource{}, nil
}
// the key is set; we need to delete it
value := string(getResp.Kvs[0].Value)
_, err = x.Etcd.Delete(ctx, key)
if err != nil {
return nil, fmt.Errorf("couldn't DELETE (%s: %s): %w", key, value, err)
}
return []dnsmessage.TXTResource{{[]string{value}}}, nil
return nil, nil
}
// soaLogMessage returns an easy-to-read string for logging SOA Answers/Authorities
@@ -1174,7 +1172,7 @@ func clientv3New(etcdEndpoint string) (*clientv3.Client, error) {
return nil, err
}
// Let's do a query to determine if etcd is really, truly there
ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
ctx, cancel := context.WithTimeout(context.Background(), etcdContextTimeout)
defer cancel()
_, err = etcdCli.Get(ctx, "some-silly-key, doesn't matter if it exists")
if err != nil {

View File

@@ -182,7 +182,7 @@ var _ = Describe("Xip", func() {
Entry("getting that value → that value", "my-key.k-v.io.", []string{"MyValue"}),
Entry("getting that value with an UPPERCASE key → that value", "MY-KEY.k-v.io.", []string{"MyValue"}),
Entry("explicitly getting that value → that value", "GeT.my-key.k-v.io.", []string{"MyValue"}),
Entry("deleting that value → the deleted value", "DelETe.my-key.k-v.io.", []string{"MyValue"}),
Entry("deleting that value → empty array", "DelETe.my-key.k-v.io.", []string{}),
Entry("getting that deleted value → empty array", "my-key.k-v.io.", []string{}),
// errors
Entry("getting a non-existent key → empty array", "nonexistent.k-v.io.", []string{}),