diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-03-06 08:21:41 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-03-06 08:21:41 +0100 |
commit | 6f582fd7501818a9828b368023b1067f71483ef4 (patch) | |
tree | b59b353fd90c9a214ece08130dab738d87e30a11 | |
parent | 196df50527a51d67c9907bdbcdddc31088054e88 (diff) | |
parent | 1b09ddc9e19c21033b7409c786be56f3283aaa0e (diff) |
Merge pull request #12452 from vespa-engine/mpolden/remove-unused-roles
Remove unused roles
14 files changed, 97 insertions, 304 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java index 6df44ceaf9d..295f8e8fd98 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java @@ -22,6 +22,14 @@ public class MockUserManagement implements UserManagement { private final Map<Role, Set<User>> memberships = new HashMap<>(); + private Set<User> get(Role role) { + var membership = memberships.get(role); + if (membership == null) { + throw new IllegalArgumentException(role + " not found"); + } + return membership; + } + @Override public void createRole(Role role) { if (memberships.containsKey(role)) @@ -40,7 +48,7 @@ public class MockUserManagement implements UserManagement { List<User> userObjs = users.stream() .map(id -> new User(id.value(), id.value(), null, null)) .collect(Collectors.toList()); - memberships.get(role).addAll(userObjs); + get(role).addAll(userObjs); } @Override @@ -52,7 +60,7 @@ public class MockUserManagement implements UserManagement { @Override public void removeUsers(Role role, Collection<UserId> users) { - memberships.get(role).removeIf(user -> users.contains(new UserId(user.email()))); + get(role).removeIf(user -> users.contains(new UserId(user.email()))); } @Override @@ -64,7 +72,7 @@ public class MockUserManagement implements UserManagement { @Override public List<User> listUsers(Role role) { - return List.copyOf(memberships.get(role)); + return List.copyOf(get(role)); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java index f5f3ebe8f35..578f516f01e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java @@ -44,9 +44,6 @@ public class Roles { /** Returns the {@link Role} the given tenant, application and role names correspond to. */ public static Role toRole(TenantName tenant, String roleName) { switch (roleName) { - case "tenantOwner": return Role.tenantOwner(tenant); - case "tenantAdmin": return Role.tenantAdmin(tenant); - case "tenantOperator": return Role.tenantOperator(tenant); case "administrator": return Role.administrator(tenant); case "developer": return Role.developer(tenant); case "reader": return Role.reader(tenant); @@ -57,10 +54,6 @@ public class Roles { /** Returns the {@link Role} the given tenant and role names correspond to. */ public static Role toRole(TenantName tenant, ApplicationName application, String roleName) { switch (roleName) { - case "applicationAdmin": return Role.applicationAdmin(tenant, application); - case "applicationOperator": return Role.applicationOperator(tenant, application); - case "applicationDeveloper": return Role.applicationDeveloper(tenant, application); - case "applicationReader": return Role.applicationReader(tenant, application); case "headless": return Role.headless(tenant, application); default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'."); } @@ -97,13 +90,6 @@ public class Roles { private static String valueOf(RoleDefinition role) { switch (role) { - case tenantOwner: return "tenantOwner"; - case tenantAdmin: return "tenantAdmin"; - case tenantOperator: return "tenantOperator"; - case applicationAdmin: return "applicationAdmin"; - case applicationOperator: return "applicationOperator"; - case applicationDeveloper: return "applicationDeveloper"; - case applicationReader: return "applicationReader"; case administrator: return "administrator"; case developer: return "developer"; case reader: return "reader"; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/InstanceRole.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/InstanceRole.java deleted file mode 100644 index 6cc726f2ac3..00000000000 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/InstanceRole.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.api.role; - -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.TenantName; - -/** - * A {@link Role} with a {@link Context} of a {@link TenantName}, an {@link ApplicationName}, and an {@link InstanceName}. - * - * @author jonmv - */ -public class InstanceRole extends Role { - - InstanceRole(RoleDefinition roleDefinition, TenantName tenant, ApplicationName application, InstanceName instance) { - super(roleDefinition, Context.limitedTo(tenant, application, instance)); - } - - /** Returns the {@link TenantName} this is bound to. */ - public TenantName tenant() { return context.tenant().get(); } - - /** Returns the {@link ApplicationName} this is bound to. */ - public ApplicationName application() { return context.application().get(); } - - /** Returns the {@link InstanceName} this is bound to. */ - public InstanceName instance() { return context.instance().get(); } - - @Override - public String toString() { - return "role '" + definition() + "' of instance '" + instance() + "' of '" + application() + "' owned by '" + tenant() + "'"; - } - -} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 4c91238453a..e549af34471 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -6,6 +6,7 @@ import com.yahoo.restapi.Path; import java.net.URI; import java.util.EnumSet; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -20,11 +21,12 @@ import java.util.Set; enum PathGroup { /** Paths exclusive to operators (including read), used for system management. */ - classifiedOperator(Optional.of("/api"), + classifiedOperator(PathPrefix.api, "/configserver/v1/{*}"), /** Paths used for system management by operators. */ - operator("/controller/v1/{*}", + operator(PathPrefix.none, + "/controller/v1/{*}", "/flags/v1/{*}", "/nodes/v2/{*}", "/orchestrator/v1/{*}", @@ -36,41 +38,41 @@ enum PathGroup { "/routing/v1/inactive/environment/{*}"), /** Paths used for creating and reading user resources. */ - user(Optional.of("/api"), + user(PathPrefix.api, "/application/v4/user", "/athenz/v1/{*}"), /** Paths used for creating tenants with proper access control. */ tenant(Matcher.tenant, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}"), /** Paths used for user management on the tenant level. */ tenantUsers(Matcher.tenant, - Optional.of("/api"), + PathPrefix.api, "/user/v1/tenant/{tenant}"), /** Paths used by tenant administrators. */ tenantInfo(Matcher.tenant, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/", "/application/v4/tenant/{tenant}/cost", "/application/v4/tenant/{tenant}/cost/{date}", "/routing/v1/status/tenant/{tenant}/{*}"), tenantKeys(Matcher.tenant, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/key/"), applicationKeys(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/key/"), /** Path for the base application resource. */ application(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}", "/application/v4/tenant/{tenant}/application/{application}/instance/", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}"), @@ -78,13 +80,13 @@ enum PathGroup { /** Paths used for user management on the application level. */ applicationUsers(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/user/v1/tenant/{tenant}/application/{application}"), /** Paths used by application administrators. */ applicationInfo(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/package", "/application/v4/tenant/{tenant}/application/{application}/deployment", "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}", @@ -108,7 +110,7 @@ enum PathGroup { developmentRestart(Matcher.tenant, Matcher.application, Matcher.instance, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/dev/region/{region}/restart", "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/perf/region/{region}/restart", "/application/v4/tenant/{tenant}/application/{application}/environment/dev/region/{region}/instance/{instance}/restart", @@ -118,7 +120,7 @@ enum PathGroup { /** Path used to restart production nodes. */ productionRestart(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/prod/region/{region}/restart", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/test/region/{region}/restart", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/staging/region/{region}/restart", @@ -130,7 +132,7 @@ enum PathGroup { developmentDeployment(Matcher.tenant, Matcher.application, Matcher.instance, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploy/{job}", "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/dev/region/{region}", "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/environment/dev/region/{region}/deploy", @@ -145,7 +147,7 @@ enum PathGroup { /** Paths used for production deployments. */ productionDeployment(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/prod/region/{region}", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/prod/region/{region}/deploy", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/environment/test/region/{region}", @@ -162,85 +164,74 @@ enum PathGroup { /** Paths used for continuous deployment to production. */ submission(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/submit", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/submit"), /** Paths used for other tasks by build services. */ // TODO: This will vanish. buildService(Matcher.tenant, Matcher.application, - Optional.of("/api"), + PathPrefix.api, "/application/v4/tenant/{tenant}/application/{application}/jobreport", "/application/v4/tenant/{tenant}/application/{application}/instance/{ignored}/jobreport"), /** Paths which contain (not very strictly) classified information about customers. */ - classifiedTenantInfo(Optional.of("/api"), + classifiedTenantInfo(PathPrefix.api, "/application/v4/", "/application/v4/tenant/"), /** Paths which contain (not very strictly) classified information about, e.g., customers. */ - classifiedInfo("/", + classifiedInfo(PathPrefix.none, + "/", "/d/{*}"), /** Same as classifiedInfo, but with optional /api prefix */ - classifiedApiInfo(Optional.of("/api"), + classifiedApiInfo(PathPrefix.api, "/deployment/v1/{*}", "/user/v1/user"), /** Paths providing public information. */ - publicInfo(Optional.of("/api"), + publicInfo(PathPrefix.api, "/badge/v1/{*}", "/zone/v1/{*}"), /** Paths used for deploying system-wide feature flags. */ - systemFlagsDeploy("/system-flags/v1/deploy"), + systemFlagsDeploy(PathPrefix.none, "/system-flags/v1/deploy"), /** Paths used for "dry-running" system-wide feature flags. */ - systemFlagsDryrun("/system-flags/v1/dryrun"); + systemFlagsDryrun(PathPrefix.none, "/system-flags/v1/dryrun"); final List<String> pathSpecs; - final String prefix; + final PathPrefix prefix; final List<Matcher> matchers; - PathGroup(String... pathSpecs) { - this(List.of(), Optional.empty(), List.of(pathSpecs)); - } - - PathGroup(Optional<String> prefix, String... pathSpecs) { + PathGroup(PathPrefix prefix, String... pathSpecs) { this(List.of(), prefix, List.of(pathSpecs)); } - PathGroup(Matcher first, String... pathSpecs) { - this(List.of(first), Optional.empty(), List.of(pathSpecs)); - } - - PathGroup(Matcher first, Optional<String> prefix, String... pathSpecs) { + PathGroup(Matcher first, PathPrefix prefix, String... pathSpecs) { this(List.of(first), prefix, List.of(pathSpecs)); } - PathGroup(Matcher first, Matcher second, String... pathSpecs) { - this(List.of(first, second), Optional.empty(), List.of(pathSpecs)); - } - - PathGroup(Matcher first, Matcher second, Optional<String> prefix, String... pathSpecs) { + PathGroup(Matcher first, Matcher second, PathPrefix prefix, String... pathSpecs) { this(List.of(first, second), prefix, List.of(pathSpecs)); } - PathGroup(Matcher first, Matcher second, Matcher third, Optional<String> prefix, String... pathSpecs) { + PathGroup(Matcher first, Matcher second, Matcher third, PathPrefix prefix, String... pathSpecs) { this(List.of(first, second, third), prefix, List.of(pathSpecs)); } /** Creates a new path group, if the given context matchers are each present exactly once in each of the given specs. */ - PathGroup(List<Matcher> matchers, Optional<String> prefix, List<String> pathSpecs) { + PathGroup(List<Matcher> matchers, PathPrefix prefix, List<String> pathSpecs) { this.matchers = matchers; - this.prefix = prefix.orElse(""); + this.prefix = prefix; this.pathSpecs = pathSpecs; } /** Returns path if it matches any spec in this group, with match groups set by the match. */ private Optional<Path> get(URI uri) { - Path matcher = new Path(uri, prefix); + Path matcher = new Path(uri, prefix.prefix); for (String spec : pathSpecs) // Iterate to be sure the Path's state is that of the match. if (matcher.matches(spec)) return Optional.of(matcher); return Optional.empty(); @@ -293,4 +284,21 @@ enum PathGroup { } + /** + * The valid prefixes of paths in a {@link PathGroup}. Provides flexibility in cases where paths are made available + * under a non-root path. + */ + enum PathPrefix { + + none(""), + api("/api"); + + private final String prefix; + + PathPrefix(String prefix) { + this.prefix = Objects.requireNonNull(prefix); + } + + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 1193db67340..55512b38f95 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -10,9 +10,11 @@ import java.util.Set; /** * Policies for REST APIs in the controller. A policy is only considered when defined in a {@link Role}. - * A policy describes a set of {@link Privilege}s, which are valid for a set of {@link SystemName}s. - * A policy is evaluated by an {@link Enforcer}, which holds the {@link SystemName} the evaluation is done in. - * A policy is evaluated with a {@link Context}, which may limit it to a specific {@link TenantName} or {@link ApplicationName}. + * + * - A policy describes a set of {@link Privilege}s, which are valid for a set of {@link SystemName}s. + * - A policy is evaluated by an {@link Enforcer}, which holds the {@link SystemName} the evaluation is done in. + * - A policy is evaluated in a {@link Context}, which may limit it to a specific {@link TenantName} or + * {@link ApplicationName}. * * @author mpolden */ @@ -25,8 +27,8 @@ enum Policy { /** Full access to everything. */ supporter(Privilege.grant(Action.read) - .on(PathGroup.all()) - .in(SystemName.all())), + .on(PathGroup.all()) + .in(SystemName.all())), /** Full access to user management for a tenant in select systems. */ tenantManager(Privilege.grant(Action.all()) @@ -103,11 +105,6 @@ enum Policy { .on(PathGroup.developmentDeployment, PathGroup.developmentRestart) .in(SystemName.all())), - /** Full access to application production deployments. */ - productionDeployment(Privilege.grant(Action.all()) - .on(PathGroup.productionDeployment) - .in(SystemName.all())), - /** Read access to all application deployments. */ deploymentRead(Privilege.grant(Action.read) .on(PathGroup.developmentDeployment, PathGroup.productionDeployment) @@ -118,11 +115,6 @@ enum Policy { .on(PathGroup.submission) .in(SystemName.all())), - /** Full access to the additional tasks needed for continuous deployment. */ - deploymentPipeline(Privilege.grant(Action.all()) // TODO remove when everyone is on new pipeline. - .on(PathGroup.buildService, PathGroup.productionRestart) - .in(SystemName.all())), - /** Read access to all information in select systems. */ classifiedRead(Privilege.grant(Action.read) .on(PathGroup.allExcept(PathGroup.classifiedOperator)) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java index 263e3284dbd..532088e94aa 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.role; import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import java.util.Objects; @@ -43,31 +42,6 @@ public abstract class Role { return new TenantRole(RoleDefinition.athenzTenantAdmin, tenant); } - /** Returns a {@link RoleDefinition#tenantPipeline} for the current system and given tenant and application. */ - public static ApplicationRole tenantPipeline(TenantName tenant, ApplicationName application) { - return new ApplicationRole(RoleDefinition.tenantPipeline, tenant, application); - } - - /** Returns a {@link RoleDefinition#athenzUser} for the current system and given tenant and application. */ - public static InstanceRole athenzUser(TenantName tenant, ApplicationName application, InstanceName instance) { - return new InstanceRole(RoleDefinition.athenzUser, tenant, application, instance); - } - - /** Returns a {@link RoleDefinition#tenantOwner} for the current system and given tenant. */ - public static TenantRole tenantOwner(TenantName tenant) { - return new TenantRole(RoleDefinition.tenantOwner, tenant); - } - - /** Returns a {@link RoleDefinition#tenantAdmin} for the current system and given tenant. */ - public static TenantRole tenantAdmin(TenantName tenant) { - return new TenantRole(RoleDefinition.tenantAdmin, tenant); - } - - /** Returns a {@link RoleDefinition#tenantOperator} for the current system and given tenant. */ - public static TenantRole tenantOperator(TenantName tenant) { - return new TenantRole(RoleDefinition.tenantOperator, tenant); - } - /** Returns a {@link RoleDefinition#reader} for the current system and given tenant. */ public static TenantRole reader(TenantName tenant) { return new TenantRole(RoleDefinition.reader, tenant); @@ -88,26 +62,6 @@ public abstract class Role { return new ApplicationRole(RoleDefinition.headless, tenant, application); } - /** Returns a {@link RoleDefinition#applicationAdmin} for the current system and given tenant and application. */ - public static ApplicationRole applicationAdmin(TenantName tenant, ApplicationName application) { - return new ApplicationRole(RoleDefinition.applicationAdmin, tenant, application); - } - - /** Returns a {@link RoleDefinition#applicationOperator} for the current system and given tenant and application. */ - public static ApplicationRole applicationOperator(TenantName tenant, ApplicationName application) { - return new ApplicationRole(RoleDefinition.applicationOperator, tenant, application); - } - - /** Returns a {@link RoleDefinition#applicationDeveloper} for the current system and given tenant and application. */ - public static ApplicationRole applicationDeveloper(TenantName tenant, ApplicationName application) { - return new ApplicationRole(RoleDefinition.applicationDeveloper, tenant, application); - } - - /** Returns a {@link RoleDefinition#applicationReader} for the current system and given tenant and application. */ - public static ApplicationRole applicationReader(TenantName tenant, ApplicationName application) { - return new ApplicationRole(RoleDefinition.applicationReader, tenant, application); - } - /** Returns a {@link RoleDefinition#buildService} for the current system and given tenant and application. */ public static ApplicationRole buildService(TenantName tenant, ApplicationName application) { return new ApplicationRole(RoleDefinition.buildService, tenant, application); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index b29b5fc2e80..c4ce70a8f1e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -32,50 +32,13 @@ public enum RoleDefinition { Policy.user, Policy.tenantCreate), - /** Application reader which can see all information about an application, its tenant and deployments. */ - applicationReader(everyone, - Policy.tenantRead, - Policy.applicationRead, - Policy.deploymentRead), - /** Build service which may submit new applications for continuous deployment. */ - buildService(applicationReader, + buildService(everyone, + Policy.tenantRead, + Policy.applicationRead, + Policy.deploymentRead, Policy.submission), - /** Application developer with access to deploy to development zones. */ - applicationDeveloper(applicationReader, - Policy.developmentDeployment), - - /** Application operator with access to normal, operational tasks of an application. */ - applicationOperator(applicationReader, - Policy.applicationOperations), - - /** Application administrator with full access to an already existing application, including emergency operations. */ - applicationAdmin(applicationDeveloper, - applicationOperator, - Policy.applicationUpdate, - Policy.applicationDelete, - Policy.applicationManager, - Policy.productionDeployment, - Policy.submission), - - /** Tenant operator with access to create application under a tenant, and to read the tenant's and public data. */ - tenantOperator(everyone, - Policy.tenantRead, - Policy.applicationCreate, - Policy.keyManagement), - - /** Tenant admin with full access to all tenant resources, except deleting the tenant. */ - tenantAdmin(tenantOperator, - applicationAdmin, - Policy.applicationDelete, - Policy.tenantManager, - Policy.tenantUpdate), - - /** Tenant admin with full access to all tenant resources. */ - tenantOwner(tenantAdmin, - Policy.tenantDelete), - /** Reader — the base role for all tenant users */ reader(Policy.tenantRead, Policy.applicationRead, @@ -99,16 +62,6 @@ public enum RoleDefinition { /** Headless — the application specific role identified by deployment keys for production */ headless(Policy.submission), - /** Build and continuous delivery service. */ // TODO replace with buildService, when everyone is on new pipeline. - tenantPipeline(everyone, - Policy.submission, - Policy.deploymentPipeline, - Policy.productionDeployment), - - /** Athenz user with access to development resources under its instances. */ - athenzUser(everyone, - Policy.developmentDeployment), - /** Tenant administrator with full access to all child resources. */ athenzTenantAdmin(everyone, Policy.tenantRead, @@ -132,12 +85,8 @@ public enum RoleDefinition { this(Set.of(), policies); } - RoleDefinition(RoleDefinition first, Policy... policies) { - this(Set.of(first), policies); - } - - RoleDefinition(RoleDefinition first, RoleDefinition second, Policy... policies) { - this(Set.of(first, second), policies); + RoleDefinition(RoleDefinition parent, Policy... policies) { + this(Set.of(parent), policies); } RoleDefinition(Set<RoleDefinition> parents, Policy... policies) { diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java index 22baedd16b4..636825573f1 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RolesTest.java @@ -29,11 +29,6 @@ public class RolesTest { Roles.toRole("hostedOperator")); assertEquals(Role.hostedSupporter(), Roles.toRole("hostedSupporter")); - assertEquals(Role.tenantOperator(tenant), - Roles.toRole("my-tenant.tenantOperator")); - assertEquals(Role.applicationReader(tenant, application), - Roles.toRole("my-tenant.my-application.applicationReader")); - assertEquals(Role.administrator(tenant), Roles.toRole("my-tenant.administrator")); assertEquals(Role.developer(tenant), Roles.toRole("my-tenant.developer")); assertEquals(Role.reader(tenant), Roles.toRole("my-tenant.reader")); @@ -42,17 +37,12 @@ public class RolesTest { @Test(expected = IllegalArgumentException.class) public void illegalTenantName() { - Roles.valueOf(Role.tenantAdmin(TenantName.from("my.tenant"))); + Roles.valueOf(Role.developer(TenantName.from("my.tenant"))); } @Test(expected = IllegalArgumentException.class) public void illegalApplicationName() { - Roles.valueOf(Role.applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app"))); - } - - @Test(expected = IllegalArgumentException.class) - public void illegalRole() { - Roles.valueOf(Role.tenantPipeline(TenantName.from("my-tenant"), ApplicationName.from("my-app"))); + Roles.valueOf(Role.headless(TenantName.from("my-tenant"), ApplicationName.from("my.app"))); } @Test(expected = IllegalArgumentException.class) diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java index 0597d9e5b2f..57b4af9d16c 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.api.role; import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; import org.junit.Test; @@ -77,70 +76,14 @@ public class RoleTest { @Test public void build_service_membership() { - Role role = Role.tenantPipeline(TenantName.from("t1"), ApplicationName.from("a1")); + Role role = Role.buildService(TenantName.from("t1"), ApplicationName.from("a1")); assertFalse(publicEnforcer.allows(role, Action.create, URI.create("/not/explicitly/defined"))); assertFalse(publicEnforcer.allows(role, Action.update, URI.create("/application/v4/tenant/t1/application/a1"))); - assertTrue(publicEnforcer.allows(role, Action.create, URI.create("/application/v4/tenant/t1/application/a1/jobreport"))); + assertTrue(publicEnforcer.allows(role, Action.create, URI.create("/application/v4/tenant/t1/application/a1/submit"))); assertFalse("No global read access", publicEnforcer.allows(role, Action.read, URI.create("/controller/v1/foo"))); } @Test - public void athenz_user_membership() { - Role role = Role.athenzUser(TenantName.from("t8"), ApplicationName.from("a6"), InstanceName.from("i1")); - assertTrue(mainEnforcer.allows(role, Action.create, URI.create("/application/v4/tenant/t8/application/a6/instance/i1/deploy/some-job"))); - assertTrue(mainEnforcer.allows(role, Action.delete, URI.create("/application/v4/tenant/t8/application/a6/instance/i1/environment/dev/region/r1"))); - assertFalse(mainEnforcer.allows(role, Action.delete, URI.create("/application/v4/tenant/t8/application/a6/instance/i1/environment/prod/region/r1"))); - } - - @Test - public void implications() { - TenantName tenant1 = TenantName.from("t1"); - ApplicationName application1 = ApplicationName.from("a1"); - TenantName tenant2 = TenantName.from("t2"); - ApplicationName application2 = ApplicationName.from("a2"); - - Role tenantOwner1 = Role.tenantOwner(tenant1); - Role tenantAdmin1 = Role.tenantAdmin(tenant1); - Role tenantAdmin2 = Role.tenantAdmin(tenant2); - Role tenantOperator1 = Role.tenantOperator(tenant1); - Role applicationAdmin11 = Role.applicationAdmin(tenant1, application1); - Role applicationOperator11 = Role.applicationOperator(tenant1, application1); - Role applicationDeveloper11 = Role.applicationDeveloper(tenant1, application1); - Role applicationReader11 = Role.applicationReader(tenant1, application1); - Role applicationReader12 = Role.applicationReader(tenant1, application2); - Role applicationReader22 = Role.applicationReader(tenant2, application2); - - assertFalse(tenantOwner1.implies(tenantOwner1)); - assertTrue(tenantOwner1.implies(tenantAdmin1)); - assertFalse(tenantOwner1.implies(tenantAdmin2)); - assertTrue(tenantOwner1.implies(tenantOperator1)); - assertTrue(tenantOwner1.implies(applicationAdmin11)); - assertTrue(tenantOwner1.implies(applicationReader11)); - assertTrue(tenantOwner1.implies(applicationReader12)); - assertFalse(tenantOwner1.implies(applicationReader22)); - - assertFalse(tenantAdmin1.implies(tenantOwner1)); - assertFalse(tenantAdmin1.implies(tenantAdmin2)); - assertTrue(tenantAdmin1.implies(applicationDeveloper11)); - - assertFalse(tenantOperator1.implies(applicationReader11)); - - assertFalse(applicationAdmin11.implies(tenantAdmin1)); - assertFalse(applicationAdmin11.implies(tenantOperator1)); - assertTrue(applicationAdmin11.implies(applicationOperator11)); - assertTrue(applicationAdmin11.implies(applicationDeveloper11)); - assertTrue(applicationAdmin11.implies(applicationReader11)); - assertFalse(applicationAdmin11.implies(applicationReader12)); - assertFalse(applicationAdmin11.implies(applicationReader22)); - - assertFalse(applicationOperator11.implies(applicationDeveloper11)); - assertTrue(applicationOperator11.implies(applicationReader11)); - - assertFalse(applicationDeveloper11.implies(applicationOperator11)); - assertTrue(applicationDeveloper11.implies(applicationReader11)); - } - - @Test public void new_implications() { TenantName tenant1 = TenantName.from("t1"); ApplicationName application1 = ApplicationName.from("a1"); @@ -191,8 +134,8 @@ public class RoleTest { // Write { var url = URI.create("/routing/v1/inactive/tenant/t1/application/a1/instance/i1/environment/prod/region/us-north-1"); - var allowedRole = Role.applicationAdmin(TenantName.from("t1"), ApplicationName.from("a1")); - var disallowedRole = Role.applicationAdmin(TenantName.from("t2"), ApplicationName.from("a2")); + var allowedRole = Role.developer(TenantName.from("t1")); + var disallowedRole = Role.developer(TenantName.from("t2")); assertTrue(allowedRole + " can override status at " + url, mainEnforcer.allows(allowedRole, Action.create, url)); assertTrue(allowedRole + " can clear status at " + url, mainEnforcer.allows(allowedRole, Action.delete, url)); assertFalse(disallowedRole + " cannot override status at " + url, mainEnforcer.allows(disallowedRole, Action.create, url)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java index 30f0d545ffe..4ae3c38bdf2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java @@ -110,7 +110,7 @@ public class AthenzRoleFilter extends JsonSecurityRequestFilterBase { futures.add(executor.submit(() -> { if ( tenant.get().type() != Tenant.Type.athenz || hasDeployerAccess(identity, ((AthenzTenant) tenant.get()).domain(), application.get())) - roleMemberships.add(Role.tenantPipeline(tenant.get().name(), application.get())); + roleMemberships.add(Role.buildService(tenant.get().name(), application.get())); })); futures.add(executor.submit(() -> { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index 66cbf4d17ef..0e3295b1143 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -335,13 +335,6 @@ public class UserApiHandler extends LoggingRequestHandler { private static String valueOf(Role role) { switch (role.definition()) { - case tenantOwner: return "tenantOwner"; - case tenantAdmin: return "tenantAdmin"; - case tenantOperator: return "tenantOperator"; - case applicationAdmin: return "applicationAdmin"; - case applicationOperator: return "applicationOperator"; - case applicationDeveloper: return "applicationDeveloper"; - case applicationReader: return "applicationReader"; case administrator: return "administrator"; case developer: return "developer"; case reader: return "reader"; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 852d4a98022..90254825f4c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -588,9 +588,11 @@ public class ApplicationApiTest extends ControllerContainerTest { // POST a 'restart application' command tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-central-1/instance/instance1/restart", POST) - .screwdriverIdentity(SCREWDRIVER_ID), + .userIdentity(HOSTED_VESPA_OPERATOR), "{\"message\":\"Requested restart of tenant1.application1.instance1 in prod.us-central-1\"}"); + addUserToHostedOperatorRole(HostedAthenzIdentities.from(SCREWDRIVER_ID)); + // POST a 'restart application' in staging environment command tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/staging/region/us-central-1/instance/instance1/restart", POST) .screwdriverIdentity(SCREWDRIVER_ID), @@ -941,10 +943,8 @@ public class ApplicationApiTest extends ControllerContainerTest { .oktaAccessToken(OKTA_AT).oktaIdentityToken(OKTA_IT), new File("instance-reference.json")); - // Grant deploy access - addScrewdriverUserToDeployRole(SCREWDRIVER_ID, - ATHENZ_TENANT_DOMAIN, - ApplicationName.from("application1")); + // Add build service to operator role + addUserToHostedOperatorRole(HostedAthenzIdentities.from(SCREWDRIVER_ID)); // POST (deploy) an application to a prod zone - allowed when project ID is not specified MultiPartStreamer entity = createApplicationDeployData(applicationPackageInstance1, true); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java index 4e06afea50d..c49f7a90194 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilterTest.java @@ -95,14 +95,14 @@ public class AthenzRoleFilterTest { assertEquals(Set.of(Role.athenzTenantAdmin(TENANT)), filter.roles(TENANT_ADMIN, APPLICATION2_CONTEXT_PATH)); - // Build services are members of the tenantPipeline role within their application subtree. + // Build services are members of the buildService role within their application subtree. assertEquals(Set.of(Role.everyone()), filter.roles(TENANT_PIPELINE, NO_CONTEXT_PATH)); assertEquals(Set.of(Role.everyone()), filter.roles(TENANT_PIPELINE, TENANT_CONTEXT_PATH)); - assertEquals(Set.of(Role.tenantPipeline(TENANT, APPLICATION)), + assertEquals(Set.of(Role.buildService(TENANT, APPLICATION)), filter.roles(TENANT_PIPELINE, APPLICATION_CONTEXT_PATH)); assertEquals(Set.of(Role.everyone()), @@ -112,7 +112,7 @@ public class AthenzRoleFilterTest { assertEquals(Set.of(Role.athenzTenantAdmin(TENANT)), filter.roles(TENANT_ADMIN_AND_PIPELINE, TENANT_CONTEXT_PATH)); - assertEquals(Set.of(Role.athenzTenantAdmin(TENANT), Role.tenantPipeline(TENANT, APPLICATION)), + assertEquals(Set.of(Role.athenzTenantAdmin(TENANT), Role.buildService(TENANT, APPLICATION)), filter.roles(TENANT_ADMIN_AND_PIPELINE, APPLICATION_CONTEXT_PATH)); // Users have nothing special under their instance diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 93d88ff8abd..6db5bc9f523 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -102,7 +102,7 @@ public class UserApiTest extends ControllerContainerCloudTest { tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", POST) .roles(Set.of(Role.administrator(TenantName.from("my-tenant")))) .data("{\"user\":\"headless@app\",\"roleName\":\"headless\"}"), - "{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"NullPointerException\"}", 500); + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"role 'headless' of 'my-app' owned by 'my-tenant' not found\"}", 400); // POST an application is allowed for a tenant developer. tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app", POST) @@ -193,10 +193,13 @@ public class UserApiTest extends ControllerContainerCloudTest { .data("{\"user\":\"administrator@tenant\",\"roleName\":\"administrator\"}"), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can't remove the last administrator of a tenant.\"}", 400); - // DELETE the tenant is available to the tenant owner. + // DELETE the tenant is not allowed tester.assertResponse(request("/application/v4/tenant/my-tenant", DELETE) - .roles(Set.of(Role.tenantOwner(id.tenant()))), - new File("tenant-without-applications.json")); + .roles(Set.of(Role.developer(id.tenant()))), + "{\n" + + " \"code\" : 403,\n" + + " \"message\" : \"Access denied\"\n" + + "}", 403); } @Test |