Validating absolute DNS name requirement on Static DNS Entry Type

CHANGELOG.md

@@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Added debugging functionality to CDN-in-a-Box for Traffic Stats.
 
 ### Fixed
+- Fixed #4743 - Validate absolute DNS name requirement on Static DNS entry for CNAME type [Related github issue](https://github.com/apache/trafficcontrol/issues/4743)
 - Fixed #4848 - `GET /api/x/cdns/capacity` gives back 500, with the message `capacity was zero`
 - Fixed #2156 - Renaming a host in TC, does not impact xmpp_id and thereby hashid [Related github issue](https://github.com/apache/trafficcontrol/issues/2156)
 - Fixed #3661 - Anonymous Proxy ipv4 whitelist does not work

docs/source/api/v2/staticdnsentries.rst

@@ -75,7 +75,7 @@ Request Structure
 
 Response Structure
 ------------------
-:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -109,7 +109,7 @@ Response Structure
 
 	{ "response": [
 		{
-			"address": "foo.bar",
+			"address": "foo.bar.",
 			"cachegroup": null,
 			"cachegroupId": null,
 			"deliveryservice": "demo1",
@@ -119,7 +119,7 @@ Response Structure
 			"lastUpdated": "2018-12-10 19:59:56+00",
 			"ttl": 300,
 			"type": "CNAME_RECORD",
-			"typeId": 40
+			"typeId": 41
 		}
 	]}
 
@@ -133,7 +133,7 @@ Creates a new, static DNS entry.
 
 Request Structure
 -----------------
-:address:      If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:      If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroupId: An optional, integer that is the :ref:`ID of a Cache Group <cache-group-id>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -155,16 +155,16 @@ Request Structure
 	Content-Type: application/json
 
 	{
-		"address": "test.quest",
+		"address": "test.quest.",
 		"deliveryserviceId": 1,
 		"host": "test",
 		"ttl": 300,
-		"typeId": 40
+		"typeId": 41
 	}
 
 Response Structure
 ------------------
-:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -203,7 +203,7 @@ Response Structure
 		}
 	],
 	"response": {
-		"address": "test.quest",
+		"address": "test.quest.",
 		"cachegroup": null,
 		"cachegroupId": null,
 		"deliveryservice": null,
@@ -212,8 +212,8 @@ Response Structure
 		"id": 2,
 		"lastUpdated": "2018-12-10 19:54:19+00",
 		"ttl": 300,
-		"type": null,
-		"typeId": 40
+		"type": "CNAME_RECORD",
+		"typeId": 41
 	}}
 
 ``PUT``
@@ -234,7 +234,7 @@ Request Structure
 	|  id  | The integral, unique identifier of the static DNS entry to modify |
 	+------+-------------------------------------------------------------------+
 
-:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroupId: An optional, integer that is the :ref:`ID of a Cache Group <cache-group-id>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -256,16 +256,16 @@ Request Structure
 	Content-Type: application/json
 
 	{
-		"address": "foo.bar",
+		"address": "foo.bar.",
 		"deliveryserviceId": 1,
 		"host": "test",
 		"ttl": 300,
-		"typeId": 40
+		"typeId": 41
 	}
 
 Response Structure
 ------------------
-:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -304,7 +304,7 @@ Response Structure
 		}
 	],
 	"response": {
-		"address": "foo.bar",
+		"address": "foo.bar.",
 		"cachegroup": null,
 		"cachegroupId": null,
 		"deliveryservice": null,
@@ -314,7 +314,7 @@ Response Structure
 		"lastUpdated": "2018-12-10 19:59:56+00",
 		"ttl": 300,
 		"type": null,
-		"typeId": 40
+		"typeId": 41
 	}}
 
 

docs/source/api/v3/staticdnsentries.rst

@@ -75,7 +75,7 @@ Request Structure
 
 Response Structure
 ------------------
-:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -109,7 +109,7 @@ Response Structure
 
 	{ "response": [
 		{
-			"address": "foo.bar",
+			"address": "foo.bar.",
 			"cachegroup": null,
 			"cachegroupId": null,
 			"deliveryservice": "demo1",
@@ -119,7 +119,7 @@ Response Structure
 			"lastUpdated": "2018-12-10 19:59:56+00",
 			"ttl": 300,
 			"type": "CNAME_RECORD",
-			"typeId": 40
+			"typeId": 41
 		}
 	]}
 
@@ -133,7 +133,7 @@ Creates a new, static DNS entry.
 
 Request Structure
 -----------------
-:address:      If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:      If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroupId: An optional, integer that is the :ref:`ID of a Cache Group <cache-group-id>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -155,16 +155,16 @@ Request Structure
 	Content-Type: application/json
 
 	{
-		"address": "test.quest",
+		"address": "test.quest.",
 		"deliveryserviceId": 1,
 		"host": "test",
 		"ttl": 300,
-		"typeId": 40
+		"typeId": 41
 	}
 
 Response Structure
 ------------------
-:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -203,7 +203,7 @@ Response Structure
 		}
 	],
 	"response": {
-		"address": "test.quest",
+		"address": "test.quest.",
 		"cachegroup": null,
 		"cachegroupId": null,
 		"deliveryservice": null,
@@ -212,8 +212,8 @@ Response Structure
 		"id": 2,
 		"lastUpdated": "2018-12-10 19:54:19+00",
 		"ttl": 300,
-		"type": null,
-		"typeId": 40
+		"type": "CNAME_RECORD",
+		"typeId": 41
 	}}
 
 ``PUT``
@@ -234,7 +234,7 @@ Request Structure
 	|  id  | The integral, unique identifier of the static DNS entry to modify |
 	+------+-------------------------------------------------------------------+
 
-:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:           If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroupId: An optional, integer that is the :ref:`ID of a Cache Group <cache-group-id>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -256,16 +256,16 @@ Request Structure
 	Content-Type: application/json
 
 	{
-		"address": "foo.bar",
+		"address": "foo.bar.",
 		"deliveryserviceId": 1,
 		"host": "test",
 		"ttl": 300,
-		"typeId": 40
+		"typeId": 41
 	}
 
 Response Structure
 ------------------
-:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server, otherwise it is the IP address to which ``host`` shall be resolved
+:address:    If ``typeId`` identifies a ``CNAME`` type record, this is the Canonical Name (CNAME) of the server with a trailing period, otherwise it is the IP address to which ``host`` shall be resolved
 :cachegroup: An optional string containing the :ref:`Name of a Cache Group <cache-group-name>` which will service this static DNS entry
 
 	.. note:: This field has no effect, and is not used by any part of Traffic Control. It exists for legacy compatibility reasons.
@@ -304,7 +304,7 @@ Response Structure
 		}
 	],
 	"response": {
-		"address": "foo.bar",
+		"address": "foo.bar.",
 		"cachegroup": null,
 		"cachegroupId": null,
 		"deliveryservice": null,
@@ -313,8 +313,8 @@ Response Structure
 		"id": 2,
 		"lastUpdated": "2018-12-10 19:59:56+00",
 		"ttl": 300,
-		"type": null,
-		"typeId": 40
+		"type": "CNAME_RECORD",
+		"typeId": 41
 	}}
 
 

traffic_ops/testing/api/v2/staticdnsentries_test.go

@@ -74,7 +74,11 @@ func UpdateTestStaticDNSEntries(t *testing.T) {
 
 func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 
-	expectedAlerts := []tc.Alerts{tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv4 address", "error"}}}, tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid DNS name", "error"}}}, tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv6 address", "error"}}}}
+	expectedAlerts := []tc.Alerts{
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv4 address", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid DNS name", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' for type: CNAME_RECORD must have a trailing period", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv6 address", "error"}}}}
 
 	// A_RECORD
 	firstStaticDNSEntry := testData.StaticDNSEntries[0]
@@ -116,6 +120,18 @@ func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[1])
 	}
 
+	//CNAME_RECORD: missing a trailing period
+	expectedAddressMissingPeriod := "cdn.test.com"
+	remoteStaticDNSEntry.Address = expectedAddressMissingPeriod
+	alert, _, status, err = TOSession.UpdateStaticDNSEntryByID(remoteStaticDNSEntry.ID, remoteStaticDNSEntry)
+	t.Log("Status Code [expect 400]: ", status)
+	if err != nil {
+		t.Logf("cannot UPDATE StaticDNSEntries using url: %v - %v\n", err, alert)
+	}
+	if !reflect.DeepEqual(alert, expectedAlerts[2]) {
+		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[2])
+	}
+
 	// AAAA_RECORD
 	thirdStaticDNSEntry := testData.StaticDNSEntries[2]
 	// Retrieve the StaticDNSEntries by name so we can get the id for the Update
@@ -131,8 +147,8 @@ func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 	if err != nil {
 		t.Logf("cannot UPDATE StaticDNSEntries using url: %v - %v\n", err, alert)
 	}
-	if !reflect.DeepEqual(alert, expectedAlerts[2]) {
-		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[2])
+	if !reflect.DeepEqual(alert, expectedAlerts[3]) {
+		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[3])
 	}
 }
 

traffic_ops/testing/api/v3/staticdnsentries_test.go

@@ -69,7 +69,7 @@ func GetTestStaticDNSEntriesIMSAfterChange(t *testing.T, header http.Header) {
 func GetTestStaticDNSEntriesIMS(t *testing.T) {
 	var header http.Header
 	header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0,0,1)
+	futureTime := time.Now().AddDate(0, 0, 1)
 	time := futureTime.Format(time.RFC1123)
 	header.Set(rfc.IfModifiedSince, time)
 
@@ -128,7 +128,11 @@ func UpdateTestStaticDNSEntries(t *testing.T) {
 
 func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 
-	expectedAlerts := []tc.Alerts{tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv4 address", "error"}}}, tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid DNS name", "error"}}}, tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv6 address", "error"}}}}
+	expectedAlerts := []tc.Alerts{
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv4 address", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid DNS name", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' for type: CNAME_RECORD must have a trailing period", "error"}}},
+		tc.Alerts{[]tc.Alert{tc.Alert{"'address' must be a valid IPv6 address", "error"}}}}
 
 	// A_RECORD
 	firstStaticDNSEntry := testData.StaticDNSEntries[0]
@@ -170,6 +174,18 @@ func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[1])
 	}
 
+	//CNAME_RECORD: missing a trailing period
+	expectedAddressMissingPeriod := "cdn.test.com"
+	remoteStaticDNSEntry.Address = expectedAddressMissingPeriod
+	alert, _, status, err = TOSession.UpdateStaticDNSEntryByID(remoteStaticDNSEntry.ID, remoteStaticDNSEntry)
+	t.Log("Status Code [expect 400]: ", status)
+	if err != nil {
+		t.Logf("cannot UPDATE StaticDNSEntries using url: %v - %v\n", err, alert)
+	}
+	if !reflect.DeepEqual(alert, expectedAlerts[2]) {
+		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[2])
+	}
+
 	// AAAA_RECORD
 	thirdStaticDNSEntry := testData.StaticDNSEntries[2]
 	// Retrieve the StaticDNSEntries by name so we can get the id for the Update
@@ -185,8 +201,8 @@ func UpdateTestStaticDNSEntriesInvalidAddress(t *testing.T) {
 	if err != nil {
 		t.Logf("cannot UPDATE StaticDNSEntries using url: %v - %v\n", err, alert)
 	}
-	if !reflect.DeepEqual(alert, expectedAlerts[2]) {
-		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[2])
+	if !reflect.DeepEqual(alert, expectedAlerts[3]) {
+		t.Errorf("got alerts: %v but expected alerts: %v", alert, expectedAlerts[3])
 	}
 }
 

traffic_ops/traffic_ops_golang/staticdnsentry/staticdnsentry.go

@@ -20,6 +20,7 @@ package staticdnsentry
  */
 
 import (
+	"fmt"
 	"net/http"
 	"strconv"
 	"time"
@@ -103,6 +104,13 @@ func (staticDNSEntry TOStaticDNSEntry) Validate() error {
 		addressErr = validation.Validate(staticDNSEntry.Address, validation.Required, is.IPv6)
 	case "CNAME_RECORD":
 		addressErr = validation.Validate(staticDNSEntry.Address, validation.Required, is.DNSName)
+		address := *staticDNSEntry.Address
+		if addressErr == nil {
+			lastChar := address[len(address)-1:]
+			if lastChar != "." {
+				addressErr = fmt.Errorf("for type: CNAME_RECORD must have a trailing period")
+			}
+		}
 	default:
 		addressErr = validation.Validate(staticDNSEntry.Address, validation.Required)
 	}

traffic_portal/app/src/common/modules/form/deliveryServiceStaticDnsEntry/form.deliveryServiceStaticDnsEntry.tpl.html

@@ -57,9 +57,20 @@
                 </div>
             </div>
             <div class="form-group" ng-class="{'has-error': hasError(dsStaticDnsEntryForm.address), 'has-feedback': hasError(dsStaticDnsEntryForm.address)}">
-                <label for="address" class="control-label col-md-2 col-sm-2 col-xs-12">Address *</label>
+                <label for="address" class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12">Address *<div class="helptooltip">
+                    <div class="helptext">The Address Rules.
+                        <ul>
+                            <li>Type:A_RECORD, address should be an IPv4 address.</li>
+                            <li>Type:AAAA_RECORD, address should be an IPv6 address.</li>
+                            <li>Type:CNAME, address must be a valid DNS name ending with a trailing period.</li>
+                            <li>Type:TXT_RECORD, address cannot be blank.</li>
+                        </ul>
+                    </div>
+                </div>
+                </label>
                 <div class="col-md-10 col-sm-10 col-xs-12">
-                    <input id="address" name="address" type="text" class="form-control" ng-model="staticDnsEntry.address" required>
+                    <input id="address" name="address" type="text" class="form-control" ng-model="staticDnsEntry.address"
+                           required title="Address must be: an IPv4, if type:A_RECORD; an IPv6, if type: AAAA_RECORD; valid DNS name ending with a trailing period, if type:CNAME_RECORD; cannot be blank, if type: TXT_RECORD">
                     <small class="input-error" ng-show="hasPropertyError(dsStaticDnsEntryForm.address, 'required')">Required</small>
                     <span ng-show="hasError(dsStaticDnsEntryForm.address)" class="form-control-feedback"><i class="fa fa-times"></i></span>
                 </div>