Merge lp:~gz/goamz/example_dns into lp:goamz

Proposed by Martin Packman
Status: Merged
Merged at revision: 40
Proposed branch: lp:~gz/goamz/example_dns
Merge into: lp:goamz
Diff against target: 29 lines (+5/-1) (has conflicts)
2 files modified
ec2/ec2t_test.go (+1/-1)
ec2/ec2test/server.go (+4/-0)
Text conflict in ec2/ec2test/server.go
To merge this branch: bzr merge lp:~gz/goamz/example_dns
Reviewer Review Type Date Requested Status
goamz maintainers Pending
Review via email: mp+179717@code.launchpad.net

Description of the change

Use testing.invalid rather than example.com

Stop using example.com addresses in testing code, which
may accidentally be used in a real lookup. Taking the
'.invalid' option from RFC 2606 means the resolve should
always fail locally.

https://codereview.appspot.com/12765043/

To post a comment you must log in.
Revision history for this message
Martin Packman (gz) wrote :

Reviewers: mp+179717_code.launchpad.net,

Message:
Please take a look.

Description:
Use testing.invalid rather than example.com

Stop using example.com addresses in testing code, which
may accidentally be used in a real lookup. Taking the
'.invalid' option from RFC 2606 means the resolve should
always fail locally.

https://code.launchpad.net/~gz/goamz/example_dns/+merge/179717

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/12765043/

Affected files:
   A [revision details]
   M ec2/ec2t_test.go
   M ec2/ec2test/server.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: ec2/ec2t_test.go
=== modified file 'ec2/ec2t_test.go'
--- ec2/ec2t_test.go 2013-01-31 17:06:10 +0000
+++ ec2/ec2t_test.go 2013-08-12 14:33:16 +0000
@@ -68,7 +68,7 @@
   })
   c.Assert(err, IsNil)
   c.Assert(inst, NotNil)
- c.Assert(inst.Instances[0].DNSName, Equals,
inst.Instances[0].InstanceId+".example.com")
+ c.Assert(inst.Instances[0].DNSName, Equals,
inst.Instances[0].InstanceId+".testing.invalid")

   id := inst.Instances[0].InstanceId

Index: ec2/ec2test/server.go
=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go 2013-01-31 17:06:10 +0000
+++ ec2/ec2test/server.go 2013-08-12 14:33:16 +0000
@@ -548,7 +548,7 @@
    InstanceId: inst.id,
    InstanceType: inst.instType,
    ImageId: inst.imageId,
- DNSName: fmt.Sprintf("%s.example.com", inst.id),
+ DNSName: fmt.Sprintf("%s.testing.invalid", inst.id),
    // TODO the rest
   }
  }

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 2013/08/12 14:38:40, gz wrote:
> Please take a look.

LGTM but wait for niemeyer.

https://codereview.appspot.com/12765043/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

What is preventing .invalid from being looked up?

[niemeyer@gopher ~]% sudo tcpdump -i wlan0 -A dst host 8.8.8.8 &
[niemeyer@gopher ~]% host foo.invalid
13:02:51.554495 IP gopher.local.44235 >
google-public-dns-a.google.com.domain: 32253+ A? foo.invalid. (29)
E..9C...@......j.......5.%..}............foo.invalid.....
13:02:51.555282 IP gopher.local.45925 >
google-public-dns-a.google.com.domain: 21752+ PTR? 8.8.8.8.in-addr.arpa.
(38)
E..B..@.@......j.....e.5..o.T............8.8.8.8.in-addr.arpa.....
13:02:51.721887 IP gopher.local.41656 >
google-public-dns-a.google.com.domain: 11674+ PTR?
106.157.168.192.in-addr.arpa. (46)
E..J..@.@......j.......5.6..-............106.157.168.192.in-addr.arpa.....
13:02:51.917257 IP gopher.local.35635 >
google-public-dns-a.google.com.domain: 45783+ A? foo.invalid. (29)
E..9C...@......j.....3.5.%...............foo.invalid.....
Host foo.invalid not found: 3(NXDOMAIN)

https://codereview.appspot.com/12765043/

Revision history for this message
Martin Packman (gz) wrote :

On 2013/08/15 16:03:41, niemeyer wrote:
> What is preventing .invalid from being looked up?

You're right, nothing really, it's just convention that it won't exist.
In practice, it fails a little faster, as it doesn't need to talk to the
example.com dns server to see if a new subdomain really exists.

https://codereview.appspot.com/12765043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ec2/ec2t_test.go'
2--- ec2/ec2t_test.go 2013-01-31 17:06:10 +0000
3+++ ec2/ec2t_test.go 2013-08-12 14:40:35 +0000
4@@ -68,7 +68,7 @@
5 })
6 c.Assert(err, IsNil)
7 c.Assert(inst, NotNil)
8- c.Assert(inst.Instances[0].DNSName, Equals, inst.Instances[0].InstanceId+".example.com")
9+ c.Assert(inst.Instances[0].DNSName, Equals, inst.Instances[0].InstanceId+".testing.invalid")
10
11 id := inst.Instances[0].InstanceId
12
13
14=== modified file 'ec2/ec2test/server.go'
15--- ec2/ec2test/server.go 2013-08-02 06:41:16 +0000
16+++ ec2/ec2test/server.go 2013-08-12 14:40:35 +0000
17@@ -548,8 +548,12 @@
18 InstanceId: inst.id,
19 InstanceType: inst.instType,
20 ImageId: inst.imageId,
21+<<<<<<< TREE
22 DNSName: fmt.Sprintf("%s.example.com", inst.id),
23 State: inst.state,
24+=======
25+ DNSName: fmt.Sprintf("%s.testing.invalid", inst.id),
26+>>>>>>> MERGE-SOURCE
27 // TODO the rest
28 }
29 }

Subscribers

People subscribed via source and target branches