Change Traffic Ops to 501 Not Implemented on unknown version reqs, and client to return a helpful message

[rob05c]

Change Traffic Ops to 501 Not Implemented on unknown version reqs, and client to return a helpful message

What does this PR do?

Change Traffic Ops to 501 Not Implemented on unknown version reqs, and client to return a helpful message

This is intended to mitigate the confusion for people running master, who try to use a newer client than their server, after a recent minor version increase.

Which TC components are affected by this PR?

• Documentation
• Grove
• Traffic Analytics
• Traffic Monitor
• Traffic Ops
• Traffic Ops ORT
• Traffic Portal
• Traffic Router
• Traffic Stats
• Traffic Vault
• Other _________

What is the best way to verify this PR?

Try to connect to a TO server since before a minor version bump (e.g. 1.3) with a client after the version increase. The client should return a helpful message.

Check all that apply

• This PR includes tests
• This PR includes documentation updates
• This PR includes an update to CHANGELOG.md
• This PR includes all required license headers
• This PR includes a database migration (ensure that migration sequence is correct)
• This PR fixes a serious security flaw. Read more: www.apache.org/security

[rob05c]

For anyone trying to test this, the easiest way I found was to manually increase the client's version, by changing traffic_ops/client/endpoints.go apiBase to /api/1.5, and then write a quick Go script to use the client against any existing TO:

package main

import (
        "fmt"
        "time"

        toclient "github.com/apache/trafficcontrol/traffic_ops/client"
)

const UserAgent = "testclient-not-implemented-resp"

func main() {
        toURI := "https://localhost"
        toUser := "yourUser"
        toPass := "yourPass"
        insecure := true
        useCache := false
        reqTimeout := time.Second * 10
        toClient, _, err := toclient.LoginWithAgent(toURI, toUser, toPass, insecure, UserAgent, useCache, reqTimeout)
        if err != nil {
                fmt.Println("Error creating client: " + err.Error())
                return
        }

        cdns, _, err := toClient.GetCDNs()
        if err != nil {
                fmt.Println("Error getting CDNs: " + err.Error())
                return
        }
        fmt.Println("Got CDNS: %+v", cdns)
}

[dangogh]

just the one message looks to be incorrect.

[rob05c]

@dangogh Er, which message?

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/2636/
Test FAILed.

[dangogh]

hmm.. I'd added a comment on session.go -- seems it got lost.. This should say "client is probably newer"... The related message in wrappers.go appears to be correct.

...this client is probably older than Traffic Ops...

[dangogh]

this should be

this client is probably newer than Traffic Ops

[rob05c]

Fixed

[asfgit]

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/2640/
Test PASSed.

[dangogh]

For anyone trying to test this, the easiest way I found was to manually increase the client's version, by changing traffic_ops/client/endpoints.go apiBase to /api/1.5, and then write a quick Go script to use the client against any existing TO:

not quite true: you need to run this against a TO that has these changes in it to produce the 501. That said, I've done that and the changes have the desired effect.

[ocket8888]

HTTP spec designates the 501 response code to be used for a lack of HTTP functionality. That more or less means that the HTTP method used in the request isn't implemented by the server (not that the method is not available for an existing resource - that would entail a 405 response). The proper response code for a path that does not exist on the server is 404 Not Found. Maybe we could use the alerts object instead?

[rob05c]

@ocket8888 It's kind of strange to comment on merged pull requests. The normal procedure, if you have concerns about code already in master, is to open a new issue.

https://github.com/apache/trafficcontrol/issues/new

[ocket8888]

I guess I'm just bummed I didn't see this before it was merged... I'll open an issue.