Allow disabling the HTTP API again.

[hanshasselberg]

If Connect is enabled, the HTTP API needs to be enabled and there is no point in not having it. If Connect is disabled however, it should be still possible to disable the HTTP API by configuring port -1 for it.
Fixes #4557.

[banks]

Great First PR 🎉

This is exactly what we all said we needed...

... but it's not going to work 😭 .

The reason this didn't check if Connect was enabled in config before (which should have been commented with hindsight) is that currently we don't require client agents to enable connect.

Our current assumption/documentation is that Connect enabled is only set on server nodes and clients just assume it's on and will fail to load certificates or similar if not.

This change will break managed proxies on some validly configured clusters where user didn't explicitly enable connect on the clients (because we told them they didn't have to).

Good news is that the problem will go away anyway soon so I don't think we should spend ages trying to fix it some more elaborate way.

I'm not quite clear on why running this block of code alters whether the HTTP API runs though - I assume it's due to that APIConfig call but I can't quickly see anything that actually changes state there. @mkeeler any insights?

[mkeeler]

@banks you are right the APIConfig call wants a valid HTTP(s) address:port to tell the managed proxies about (or watches but that is a different problem not covered here)

Maybe the better short term solution is then to attempt to get the api config at the start and use that as a condition to enabling the proxy manager.

Something like:

if !c.ConnectTestDisableManagedProxies {
   if acfg, err := a.config.APIConfig(true); err == nil {
      a.proxyManager = proxy.NewManager()
      a.proxyManager.AllowRoot = a.config.ConnectProxyAllowManagedRoot
      a.proxyManager.State = a.State
      a.proxyManager.Logger = a.logger
      if a.config.DataDir != "" {
         // DataDir is required for all non-dev mode agents, but we want
         // to allow setting the data dir for demos and so on for the agent,
         // so do the check above instead.
         a.proxyManager.DataDir = filepath.Join(a.config.DataDir, "proxy")

         // Restore from our snapshot (if it exists)
         if err := a.proxyManager.Restore(a.proxyManager.SnapshotPath()); err != nil {
            a.logger.Printf("[WARN] agent: error restoring proxy state: %s", err)
         }
      }
      
      a.proxyManager.ProxyEnv = acfg.GenerateEnv()
      go a.proxyManager.Run()
   } else {
      a.logger.Printf("[INFO] agent: Connect managed proxies are disabled due to disabling the HTTP API")
   }
}

[hanshasselberg]

@banks @mkeeler thank you for your reviews! I did it the way you proposed and now consul again accepts an invalid http configuration and won't start connect managed proxies if that is the case.

[mkeeler]

Looks good to me