Skip to content

Commit

Permalink
Merge pull request kubernetes-retired#368 from mumoshu/drop-deprecate…
Browse files Browse the repository at this point in the history
…d-hosted-zone

Drop deprecated `hostedZone`(not `hostedZoneId`) in cluster.yaml
  • Loading branch information
mumoshu authored Mar 1, 2017
2 parents 54eab73 + 37cc49b commit 6c5b6ec
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 80 deletions.
42 changes: 1 addition & 41 deletions core/controlplane/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,46 +381,6 @@ func (c *ClusterRef) validateDNSConfig(r53 r53Service) error {
return nil
}

if c.HostedZoneID == "" {
//TODO(colhom): When HostedZone parameter is gone, this block can be removed
//Config will gaurantee that HostedZoneID is set from the get-go
listHostedZoneInput := route53.ListHostedZonesByNameInput{
DNSName: aws.String(c.HostedZone),
}

zonesResp, err := r53.ListHostedZonesByName(&listHostedZoneInput)
if err != nil {
return fmt.Errorf("Error validating HostedZone: %s", err)
}

zones := zonesResp.HostedZones

if len(zones) == 0 {
return fmt.Errorf("hosted zone %s does not exist", c.HostedZone)
}

var matchingZone *route53.HostedZone
for _, zone := range zones {
if aws.StringValue(zone.Name) == c.HostedZone {
if matchingZone != nil {
//This means we've found another match, and HostedZone is ambiguous
return fmt.Errorf("multiple hosted-zones found for DNS name \"%s\"", c.HostedZone)
}
matchingZone = zone
} else {
/* Weird API semantics: if we see a zone which doesn't match the name
we've exhausted all zones which match the name
http://docs.aws.amazon.com/cli/latest/reference/route53/list-hosted-zones-by-name.html#options */

break
}
}
if matchingZone == nil {
return fmt.Errorf("hosted zone %s does not exist", c.HostedZone)
}
c.HostedZoneID = aws.StringValue(matchingZone.Id)
}

hzOut, err := r53.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(c.HostedZoneID)})
if err != nil {
return fmt.Errorf("error getting hosted zone %s: %v", c.HostedZoneID, err)
Expand All @@ -445,7 +405,7 @@ func (c *ClusterRef) validateDNSConfig(r53 r53Service) error {
return fmt.Errorf(
"RecordSet for \"%s\" already exists in Hosted Zone \"%s.\"",
c.ExternalDNSName,
c.HostedZone,
c.HostedZoneID,
)
}
}
Expand Down
18 changes: 1 addition & 17 deletions core/controlplane/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,6 @@ func TestValidateDNSConfig(t *testing.T) {
`
createRecordSet: true
recordSetTTL: 60
hostedZone: core-os.net
`, `
createRecordSet: true
recordSetTTL: 60
hostedZoneId: staging_id_1
`, `
createRecordSet: true
Expand All @@ -392,31 +388,19 @@ hostedZoneId: /hostedzone/staging_id_2
`
createRecordSet: true
recordSetTTL: 60
hostedZone: staging.core-os.net # hostedZone is ambiguous
`, `
createRecordSet: true
recordSetTTL: 60
hostedZoneId: /hostedzone/staging_id_3 # <staging_id_id> is not a super-domain
`, `
createRecordSet: true
recordSetTTL: 60
hostedZone: zebras.coreos.com # zebras.coreos.com is not a super-domain
`, `
createRecordSet: true
recordSetTTL: 60
hostedZoneId: /hostedzone/staging_id_5 #non-existant hostedZoneId
`, `
createRecordSet: true
recordSetTTL: 60
hostedZone: unicorns.core-os.net #non-existant hostedZone DNS name
`,
}

for _, validConfig := range validDNSConfigs {
configBody := defaultConfigValues(t, validConfig)
clusterConfig, err := config.ClusterFromBytes([]byte(configBody))
if err != nil {
t.Errorf("could not get valid cluster config: %v", err)
t.Errorf("could not get valid cluster config in `%s`: %v", configBody, err)
continue
}
c := &ClusterRef{Cluster: clusterConfig}
Expand Down
22 changes: 8 additions & 14 deletions core/controlplane/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,6 @@ func ConfigFromBytes(data []byte) (*Config, error) {
}

func (c *Cluster) Load() error {
// HostedZone needs to end with a '.', amazon will not append it for you.
// as it will with RecordSets
c.HostedZone = WithTrailingDot(c.HostedZone)

// If the user specified no subnets, we assume that a single AZ configuration with the default instanceCIDR is demanded
if len(c.Subnets) == 0 && c.InstanceCIDR == "" {
c.InstanceCIDR = "10.0.0.0/24"
Expand Down Expand Up @@ -420,7 +416,6 @@ type Cluster struct {
RecordSetTTL int `yaml:"recordSetTTL,omitempty"`
TLSCADurationDays int `yaml:"tlsCADurationDays,omitempty"`
TLSCertDurationDays int `yaml:"tlsCertDurationDays,omitempty"`
HostedZone string `yaml:"hostedZone,omitempty"`
HostedZoneID string `yaml:"hostedZoneId,omitempty"`
ProvidedEncryptService EncryptService
CustomSettings map[string]interface{} `yaml:"customSettings,omitempty"`
Expand Down Expand Up @@ -787,15 +782,8 @@ func (c Config) InternetGatewayRef() string {

func (c Cluster) valid() error {
if c.CreateRecordSet {
if c.HostedZone == "" && c.HostedZoneID == "" {
return errors.New("hostedZone or hostedZoneID must be specified createRecordSet is true")
}
if c.HostedZone != "" && c.HostedZoneID != "" {
return errors.New("hostedZone and hostedZoneID cannot both be specified")
}

if c.HostedZone != "" {
fmt.Printf("Warning: the 'hostedZone' parameter is deprecated. Use 'hostedZoneId' instead\n")
if c.HostedZoneID == "" {
return errors.New("hostedZoneID must be specified when createRecordSet is true")
}

if c.RecordSetTTL < 1 {
Expand All @@ -807,6 +795,12 @@ func (c Cluster) valid() error {
"recordSetTTL should not be modified when createRecordSet is false",
)
}

if c.HostedZoneID != "" {
return errors.New(
"hostedZoneId should not be modified when createRecordSet is false",
)
}
}

var dnsServiceIPAddr net.IP
Expand Down
16 changes: 8 additions & 8 deletions core/controlplane/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,11 @@ routeTableId: rtb-xxxxxx
vpcId: vpc-xxxxx
`, `
createRecordSet: false
hostedZone: ""
hostedZoneId: ""
`, `
createRecordSet: true
recordSetTTL: 400
hostedZone: core-os.net
`, `
createRecordSet: true
hostedZone: "staging.core-os.net"
hostedZoneId: "XXXXXXXXXXX"
`, `
createRecordSet: true
hostedZoneId: "XXXXXXXXXXX"
Expand Down Expand Up @@ -118,10 +115,13 @@ createRecordSet: true
createRecordSet: false
recordSetTTL: 400
`, `
createRecordSet: true
recordSetTTL: 60
hostedZone: staging.core-os.net
# hostedZoneId should'nt be modified when createRecordSet is false
createRecordSet: false
hostedZoneId: /hostedzone/staging_id_2 #hostedZone and hostedZoneId defined
`, `
# hostedZone had been deprecated and then dropped
createRecordSet: true
hostedZone: "staging.core-os.net"
`,
}

Expand Down

0 comments on commit 6c5b6ec

Please sign in to comment.