diff options
6 files changed, 46 insertions, 48 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/AuthMethod.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/AuthMethod.java index 88b8a05c4c6..23951aeea37 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/AuthMethod.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/AuthMethod.java @@ -8,7 +8,13 @@ package com.yahoo.config.provision.zone; */ public enum AuthMethod { + /** Clients can authenticate with a certificate (mutual TLS) */ mtls, + + /** Clients can authenticate with a secret token */ token, + /** Clients cannot authenticate with the endpoint directly */ + none; + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 1272bf4d00d..4e6a2946ec7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -176,9 +176,7 @@ public class RoutingController { /** Returns the zone- and region-scoped endpoints of given deployment */ public EndpointList endpointsOf(DeploymentId deployment, ClusterSpec.Id cluster, List<GeneratedEndpoint> generatedEndpoints) { - // TODO(mpolden): Support tokens only when generated endpoints are available - boolean tokenSupported = tokenEndpointEnabled(deployment.applicationId()) && - (generatedEndpoints.isEmpty() || generatedEndpoints.stream().anyMatch(ge -> ge.authMethod() == AuthMethod.token)); + boolean tokenSupported = tokenEndpointEnabled(deployment.applicationId()) && generatedEndpoints.stream().anyMatch(ge -> ge.authMethod() == AuthMethod.token); RoutingMethod routingMethod = controller.zoneRegistry().routingMethod(deployment.zoneId()); boolean isProduction = deployment.zoneId().environment().isProduction(); List<Endpoint> endpoints = new ArrayList<>(); @@ -187,9 +185,6 @@ public class RoutingController { .on(Port.fromRoutingMethod(routingMethod)) .target(cluster, deployment); endpoints.add(zoneEndpoint.in(controller.system())); - if (tokenSupported) { - endpoints.add(zoneEndpoint.authMethod(AuthMethod.token).in(controller.system())); - } Endpoint.EndpointBuilder regionEndpoint = Endpoint.of(deployment.applicationId()) .routingMethod(routingMethod) .on(Port.fromRoutingMethod(routingMethod)) @@ -203,11 +198,17 @@ public class RoutingController { boolean include = switch (generatedEndpoint.authMethod()) { case token -> tokenSupported; case mtls -> true; + case none -> false; }; if (include) { - endpoints.add(zoneEndpoint.generatedFrom(generatedEndpoint).in(controller.system())); - if (isProduction) { - endpoints.add(regionEndpoint.generatedFrom(generatedEndpoint).in(controller.system())); + endpoints.add(zoneEndpoint.generatedFrom(generatedEndpoint) + .authMethod(generatedEndpoint.authMethod()) + .in(controller.system())); + // Only a single region endpoint is needed, not one per auth method + if (isProduction && generatedEndpoint.authMethod() == AuthMethod.mtls) { + endpoints.add(regionEndpoint.generatedFrom(generatedEndpoint) + .authMethod(AuthMethod.none) + .in(controller.system())); } } } @@ -241,7 +242,7 @@ public class RoutingController { .routingMethod(method); endpoints.add(builder.in(controller.system())); for (var ge : generatedEndpoints.cluster(cluster)) { - endpoints.add(builder.generatedFrom(ge).in(controller.system())); + endpoints.add(builder.generatedFrom(ge).authMethod(ge.authMethod()).in(controller.system())); } } return EndpointList.copyOf(endpoints); @@ -261,7 +262,7 @@ public class RoutingController { List<Endpoint> endpoints = new ArrayList<>(); endpoints.add(builder.in(controller.system())); for (var ge : generatedEndpoints.cluster(cluster)) { - endpoints.add(builder.generatedFrom(ge).in(controller.system())); + endpoints.add(builder.generatedFrom(ge).authMethod(ge.authMethod()).in(controller.system())); } return EndpointList.copyOf(endpoints); } @@ -437,7 +438,11 @@ public class RoutingController { return List.of(); } return Arrays.stream(AuthMethod.values()) - .filter(method -> method != AuthMethod.token || token) + .filter(method -> switch (method) { + case token -> token; + case mtls -> true; + case none -> false; + }) .map(method -> new GeneratedEndpoint(GeneratedEndpoint.createPart(controller.random(true)), applicationPart, method)) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index 010bc023dad..7d25f58a1a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -558,6 +558,7 @@ public class Endpoint { this.cluster = cluster; this.scope = requireUnset(Scope.weighted); this.targets = List.of(new Target(new DeploymentId(application.instance(instance.get()), effectiveZone(zone)))); + this.authMethod = AuthMethod.none; return this; } @@ -594,18 +595,13 @@ public class Endpoint { /** Sets the generated ID to use when building this */ public EndpointBuilder generatedFrom(GeneratedEndpoint generated) { this.generated = Optional.of(generated); - this.authMethod = generated.authMethod(); return this; } /** Sets the system that owns this */ public Endpoint in(SystemName system) { - if (system.isPublic() && routingMethod != RoutingMethod.exclusive) { - throw new IllegalArgumentException("Public system only supports routing method " + RoutingMethod.exclusive); - } - String prefix = authMethod == AuthMethod.token ? "token-" : ""; String name = endpointOrClusterAsString(endpointId, Objects.requireNonNull(cluster, "cluster must be non-null")); - URI url = createUrl(prefix + name, + URI url = createUrl(name, Objects.requireNonNull(application, "application must be non-null"), Objects.requireNonNull(instance, "instance must be non-null"), Objects.requireNonNull(targets, "targets must be non-null"), @@ -614,8 +610,20 @@ public class Endpoint { Objects.requireNonNull(port, "port must be non-null"), Objects.requireNonNull(generated) ); + if (system.isPublic() && routingMethod != RoutingMethod.exclusive) { + throw illegal(url, "Public system only supports routing method " + RoutingMethod.exclusive + ", got " + routingMethod); + } if (routingMethod.isDirect() && !port.isDefault()) { - throw new IllegalArgumentException("Routing method " + routingMethod + " can only use default port"); + throw illegal(url, "Routing method " + routingMethod + " can only use default port, got " + port); + } + if (authMethod == AuthMethod.token && generated.isEmpty()) { + throw illegal(url, authMethod + " is only supported for generated endpoints"); + } + if (scope != Scope.weighted && generated.isPresent() && generated.get().authMethod() != authMethod) { + throw illegal(url, "Authentication method of " + scope + " endpoint does not match authentication method of generated endpoint: " + generated.get().authMethod()); + } + if ((scope == Scope.weighted) != (authMethod == AuthMethod.none)) { + throw illegal(url, "Attempted to set unsupported authentication method " + authMethod + " on " + scope + " endpoint"); } return new Endpoint(application, instance, @@ -632,6 +640,10 @@ public class Endpoint { generated); } + private static IllegalArgumentException illegal(URI url, String reason) { + return new IllegalArgumentException("Invalid endpoint: " + url + ": " + reason); + } + private Scope requireUnset(Scope scope) { if (this.scope != null) { throw new IllegalArgumentException("Cannot change endpoint scope. Already set to " + scope); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java index 9bc9403b9d6..b490995b9ec 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RoutingPolicySerializer.java @@ -136,6 +136,7 @@ public class RoutingPolicySerializer { return switch (authMethod) { case token -> "token"; case mtls -> "mtls"; + case none -> "none"; }; } @@ -143,6 +144,7 @@ public class RoutingPolicySerializer { return switch (field.asString()) { case "token" -> AuthMethod.token; case "mtls" -> AuthMethod.mtls; + case "none" -> AuthMethod.none; default -> throw new IllegalArgumentException("Unknown auth method '" + field.asString() + "'"); }; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 2a2c544f44d..7ac9b034900 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -422,6 +422,7 @@ public class RoutingPolicies { boolean skipBasedOnAuthMethod = switch (endpoint.authMethod()) { case token -> true; case mtls -> false; + case none -> true; }; if (skipBasedOnAuthMethod) return; if (endpoint.routingMethod() != RoutingMethod.exclusive) return; // Not supported for this routing method diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index b9da87771c0..835243fdc01 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -341,32 +341,6 @@ public class RoutingPoliciesTest { } @Test - void zone_token_endpoints() { - var tester = new RoutingPoliciesTester(); - tester.enableTokenEndpoint(true); - - var context1 = tester.newDeploymentContext("tenant1", "app1", "default"); - - // Deploy application - ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) - .region(zone2.region()) - .container("c0", AuthMethod.mtls, AuthMethod.token) - .build(); - tester.provisionLoadBalancers(1, context1.instanceId(), false, zone1, zone2); - context1.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.prod).deploy(); - - // Deployment creates records and policies for all clusters in all zones - List<String> expectedRecords = List.of( - "c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "c0.app1.tenant1.us-west-1.vespa.oath.cloud", - "token-c0.app1.tenant1.us-central-1.vespa.oath.cloud", - "token-c0.app1.tenant1.us-west-1.vespa.oath.cloud" - ); - assertEquals(expectedRecords, tester.recordNames()); - assertEquals(2, tester.policiesOf(context1.instanceId()).size()); - } - - @Test void zone_routing_policies_with_shared_routing() { var tester = new RoutingPoliciesTester(new DeploymentTester(), false); var context = tester.newDeploymentContext("tenant1", "app1", "default"); @@ -1074,9 +1048,7 @@ public class RoutingPoliciesTest { "cbff1506.cafed00d.z.vespa-app.cloud", "d151139b.cafed00d.z.vespa-app.cloud", "foo.app1.tenant1.g.vespa-app.cloud", - "foo.cafed00d.g.vespa-app.cloud", - "token-c1.app1.tenant1.aws-eu-west-1a.z.vespa-app.cloud", - "token-c1.app1.tenant1.aws-us-east-1c.z.vespa-app.cloud" + "foo.cafed00d.g.vespa-app.cloud" ); assertEquals(expectedRecords, tester.recordNames()); assertEquals(4, tester.policiesOf(context.instanceId()).size()); |