sds: don't throw exceptions in initialize()

[htuch]

Similar root cause to #4377 - we can't throw exceptions in initialize().

Fixes #10976

Risk level: Low
Testing: Unit/integration test added.

Signed-off-by: Harvey Tuch htuch@google.com

[snowp]

Seems right to me, just one comment

[htuch]

@snowp turns out there is a slight problem in moving the subscription config to the constructor. Check out this config fragment from one of the integration tests:

static_resources:
  clusters:
    - name: cluster_0
      connect_timeout: 5s
      transport_socket:
        name: envoy.transport_sockets.tls
        typed_config:
          "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
          common_tls_context:
            tls_certificates:
              - certificate_chain:
                  filename: /b/f/w/bazel-out/k8-fastbuild/bin/test/integration/sds_dynamic_integration_test.runfiles/envoy/test/config/integration/certs/clientcert.pem
                private_key:
                  filename: /b/f/w/bazel-out/k8-fastbuild/bin/test/integration/sds_dynamic_integration_test.runfiles/envoy/test/config/integration/certs/clientkey.pem
            validation_context_sds_secret_config:
              name: validation_secret
              sds_config:
                api_config_source:
                  api_type: GRPC
                  grpc_services:
                    - envoy_grpc:
                        cluster_name: sds_cluster
      load_assignment:
        cluster_name: cluster_0
        endpoints:
          - lb_endpoints:
              - endpoint:
                  address:
                    socket_address:
                      address: 127.0.0.1
                      port_value: 36887
    - name: sds_cluster
      connect_timeout: 5s
      http2_protocol_options:
        {}
      load_assignment:
        cluster_name: cluster_0
        endpoints:
          - lb_endpoints:
              - endpoint:
                  address:
                    socket_address:
                      address: 127.0.0.1
                      port_value: 42657

cluster_0 refers to sds_cluster via its transport socket, but we see an error:

[2020-05-18 00:30:06.019][24][critical][main] [source/server/server.cc:99] error initializing configuration '/b/f/w/_tmp/d0a45f39a3cb27a1acdbf12470d6dce1/bootstrap.json': envoy.config.core.v3.ApiConfigSource must have a statically defined non-EDS cluster: 'sds_cluster' does not exist, was added via api, or is an EDS cluster

because sds_cluster is defined after cluster_0, with the check occurring before sds_cluster is processed.

I think the "cleanest" solution here might be to have ClusterManager be aware of the set of pending config fragments that are being considered for loading in the current stack frame, e.g. pendingClusters(), and have

envoy/source/common/config/utility.cc

Line 139 in cf2d4df

void Utility::validateClusterName(const Upstream::ClusterManager::ClusterInfoMap& clusters,

be able to query this. WDYT?

[snowp]

I like that idea, hopefully it will future proof us against other new uses cases where we need to reference a static cluster.

[htuch]

@snowp this is now updated, PTAL.

[snowp]

Thanks!