[ONOS-7093] Add ISSU permissions
Change-Id: I9097019cf4b42d41817daafe3ce9ad8644ccb148
diff --git a/core/api/src/main/java/org/onosproject/security/AppPermission.java b/core/api/src/main/java/org/onosproject/security/AppPermission.java
index b79bc7e..c0f7fe9 100644
--- a/core/api/src/main/java/org/onosproject/security/AppPermission.java
+++ b/core/api/src/main/java/org/onosproject/security/AppPermission.java
@@ -82,7 +82,10 @@
TUNNEL_WRITE,
TUNNEL_EVENT,
UI_READ,
- UI_WRITE
+ UI_WRITE,
+ UPGRADE_READ,
+ UPGRADE_WRITE,
+ UPGRADE_EVENT
}
protected Type type;
diff --git a/core/api/src/main/java/org/onosproject/upgrade/UpgradeService.java b/core/api/src/main/java/org/onosproject/upgrade/UpgradeService.java
index ba5889c..fec401f 100644
--- a/core/api/src/main/java/org/onosproject/upgrade/UpgradeService.java
+++ b/core/api/src/main/java/org/onosproject/upgrade/UpgradeService.java
@@ -27,6 +27,13 @@
extends ListenerService<UpgradeEvent, UpgradeEventListener> {
/**
+ * Returns the current upgrade state.
+ *
+ * @return the current upgrade state
+ */
+ Upgrade getState();
+
+ /**
* Returns whether an upgrade is in progress.
* <p>
* An upgrade is in progress if the upgrade {@link Upgrade.Status} is active, e.g.
@@ -37,13 +44,6 @@
boolean isUpgrading();
/**
- * Returns the current upgrade state.
- *
- * @return the current upgrade state
- */
- Upgrade getState();
-
- /**
* Returns the currently active software version.
* <p>
* The returned version is representative of the version currently in control of the network. When the upgrade
diff --git a/core/net/src/main/java/org/onosproject/upgrade/impl/UpgradeManager.java b/core/net/src/main/java/org/onosproject/upgrade/impl/UpgradeManager.java
index 9fdd7a2..7a83724 100644
--- a/core/net/src/main/java/org/onosproject/upgrade/impl/UpgradeManager.java
+++ b/core/net/src/main/java/org/onosproject/upgrade/impl/UpgradeManager.java
@@ -48,6 +48,8 @@
import org.onosproject.upgrade.UpgradeService;
import org.slf4j.Logger;
+import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.*;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -79,8 +81,8 @@
private Version localVersion;
private AtomicValue<Upgrade> state;
private final AtomicReference<Upgrade> currentState = new AtomicReference<>();
- private final AtomicValueEventListener<Upgrade> stateListener = event -> handleUpgradeEvent(event);
- private final ClusterEventListener clusterListener = event -> handleClusterEvent(event);
+ private final AtomicValueEventListener<Upgrade> stateListener = this::handleUpgradeEvent;
+ private final ClusterEventListener clusterListener = this::handleClusterEvent;
@Activate
public void activate() {
@@ -92,12 +94,11 @@
localVersion = versionService.version();
currentState.set(state.get());
- if (currentState.get() == null) {
- currentState.set(new Upgrade(localVersion, localVersion, Upgrade.Status.INACTIVE));
- state.set(currentState.get());
+ if (getState() == null) {
+ initializeState(new Upgrade(localVersion, localVersion, Upgrade.Status.INACTIVE));
}
- Upgrade upgrade = currentState.get();
+ Upgrade upgrade = getState();
// If the upgrade state is not initialized, ensure this node matches the version of the cluster.
if (!upgrade.status().active() && !Objects.equals(upgrade.source(), localVersion)) {
@@ -110,9 +111,9 @@
if (upgrade.status() == Upgrade.Status.INITIALIZED) {
// If the source version equals the target version, attempt to update the target version.
if (Objects.equals(upgrade.source(), upgrade.target()) && !Objects.equals(upgrade.target(), localVersion)) {
+ checkPermission(UPGRADE_WRITE);
upgrade = new Upgrade(upgrade.source(), localVersion, upgrade.status());
- currentState.set(upgrade);
- state.set(upgrade);
+ initializeState(upgrade);
}
}
@@ -142,19 +143,53 @@
log.info("Stopped");
}
+ /**
+ * Initializes the state when the cluster starts.
+ * <p>
+ * This method must be called when updating the state in order to check the permissions
+ *
+ * @param newState new state
+ */
+ private void initializeState(Upgrade newState) {
+ checkPermission(UPGRADE_WRITE);
+ currentState.set(newState);
+ state.set(newState);
+ }
+
+ /**
+ * Changes the current state to new one.
+ * <p>
+ * This method must be called when changing between states in order to check the permissions and
+ * to avoid concurrent state modifications
+ *
+ * @param oldState current upgrade state
+ * @param newState new upgrade state
+ *
+ * @throws IllegalStateException if an upgrade is already in progress
+ */
+ private void changeState(Upgrade oldState, Upgrade newState) {
+ checkPermission(UPGRADE_WRITE);
+ if (!state.compareAndSet(oldState, newState)) {
+ throw new IllegalStateException("Concurrent upgrade modification");
+ } else {
+ currentState.set(newState);
+ }
+ }
+
+ @Override
+ public Upgrade getState() {
+ checkPermission(UPGRADE_READ);
+ return currentState.get();
+ }
+
@Override
public boolean isUpgrading() {
return getState().status().active();
}
@Override
- public Upgrade getState() {
- return currentState.get();
- }
-
- @Override
public Version getVersion() {
- Upgrade upgrade = currentState.get();
+ Upgrade upgrade = getState();
return upgrade.status().upgraded()
? upgrade.target()
: upgrade.source();
@@ -167,7 +202,7 @@
@Override
public boolean isLocalUpgraded() {
- Upgrade upgrade = currentState.get();
+ Upgrade upgrade = getState();
return upgrade.status().active()
&& !upgrade.source().equals(upgrade.target())
&& localVersion.equals(upgrade.target());
@@ -175,7 +210,7 @@
@Override
public void initialize() {
- Upgrade inactive = currentState.get();
+ Upgrade inactive = getState();
// If the current upgrade status is active, fail initialization.
if (inactive.status().active()) {
@@ -187,27 +222,19 @@
localVersion,
localVersion,
Upgrade.Status.INITIALIZING);
- if (!state.compareAndSet(inactive, initializing)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(initializing);
+ changeState(inactive, initializing);
- // Set the upgrade status to INITIALIZED.
- Upgrade initialized = new Upgrade(
- initializing.source(),
- initializing.target(),
- Upgrade.Status.INITIALIZED);
- if (!state.compareAndSet(initializing, initialized)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(initialized);
- }
- }
+ // Set the upgrade status to INITIALIZED.
+ Upgrade initialized = new Upgrade(
+ initializing.source(),
+ initializing.target(),
+ Upgrade.Status.INITIALIZED);
+ changeState(initializing, initialized);
}
@Override
public void upgrade() {
- Upgrade initialized = currentState.get();
+ Upgrade initialized = getState();
// If the current upgrade status is not INITIALIZED, throw an exception.
if (initialized.status() != Upgrade.Status.INITIALIZED) {
@@ -219,27 +246,19 @@
initialized.source(),
initialized.target(),
Upgrade.Status.UPGRADING);
- if (!state.compareAndSet(initialized, upgrading)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(upgrading);
+ changeState(initialized, upgrading);
- // Set the upgrade status to UPGRADED.
- Upgrade upgraded = new Upgrade(
- upgrading.source(),
- upgrading.target(),
- Upgrade.Status.UPGRADED);
- if (!state.compareAndSet(upgrading, upgraded)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(upgraded);
- }
- }
+ // Set the upgrade status to UPGRADED.
+ Upgrade upgraded = new Upgrade(
+ upgrading.source(),
+ upgrading.target(),
+ Upgrade.Status.UPGRADED);
+ changeState(upgrading, upgraded);
}
@Override
public void commit() {
- Upgrade upgraded = currentState.get();
+ Upgrade upgraded = getState();
// If the current upgrade status is not UPGRADED, throw an exception.
if (upgraded.status() != Upgrade.Status.UPGRADED) {
@@ -260,38 +279,26 @@
upgraded.source(),
upgraded.target(),
Upgrade.Status.COMMITTING);
- if (!state.compareAndSet(upgraded, committing)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(committing);
+ changeState(upgraded, committing);
- // Set the upgrade status to COMMITTED.
- Upgrade committed = new Upgrade(
- committing.source(),
- committing.target(),
- Upgrade.Status.COMMITTED);
- if (!state.compareAndSet(committing, committed)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(committed);
+ // Set the upgrade status to COMMITTED.
+ Upgrade committed = new Upgrade(
+ committing.source(),
+ committing.target(),
+ Upgrade.Status.COMMITTED);
+ changeState(committing, committed);
- // Set the upgrade status to INACTIVE.
- Upgrade inactive = new Upgrade(
- localVersion,
- localVersion,
- Upgrade.Status.INACTIVE);
- if (!state.compareAndSet(committed, inactive)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(inactive);
- }
- }
- }
+ // Set the upgrade status to INACTIVE.
+ Upgrade inactive = new Upgrade(
+ localVersion,
+ localVersion,
+ Upgrade.Status.INACTIVE);
+ changeState(committed, inactive);
}
@Override
public void rollback() {
- Upgrade upgraded = currentState.get();
+ Upgrade upgraded = getState();
// If the current upgrade status is not UPGRADED, throw an exception.
if (upgraded.status() != Upgrade.Status.UPGRADED) {
@@ -303,27 +310,19 @@
upgraded.source(),
upgraded.target(),
Upgrade.Status.ROLLING_BACK);
- if (!state.compareAndSet(upgraded, rollingBack)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(rollingBack);
+ changeState(upgraded, rollingBack);
- // Set the upgrade status to ROLLED_BACK.
- Upgrade rolledBack = new Upgrade(
- rollingBack.source(),
- rollingBack.target(),
- Upgrade.Status.ROLLED_BACK);
- if (!state.compareAndSet(rollingBack, rolledBack)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(rolledBack);
- }
- }
+ // Set the upgrade status to ROLLED_BACK.
+ Upgrade rolledBack = new Upgrade(
+ rollingBack.source(),
+ rollingBack.target(),
+ Upgrade.Status.ROLLED_BACK);
+ changeState(rollingBack, rolledBack);
}
@Override
public void reset() {
- Upgrade upgraded = currentState.get();
+ Upgrade upgraded = getState();
// If the current upgrade status is not INITIALIZED or ROLLED_BACK, throw an exception.
if (upgraded.status() != Upgrade.Status.INITIALIZED
@@ -345,33 +344,21 @@
upgraded.source(),
upgraded.target(),
Upgrade.Status.RESETTING);
- if (!state.compareAndSet(upgraded, resetting)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(resetting);
+ changeState(upgraded, resetting);
- // Set the upgrade status to RESET.
- Upgrade reset = new Upgrade(
- resetting.source(),
- resetting.target(),
- Upgrade.Status.RESET);
- if (!state.compareAndSet(resetting, reset)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(reset);
+ // Set the upgrade status to RESET.
+ Upgrade reset = new Upgrade(
+ resetting.source(),
+ resetting.target(),
+ Upgrade.Status.RESET);
+ changeState(resetting, reset);
- // Set the upgrade status to INACTIVE.
- Upgrade inactive = new Upgrade(
- localVersion,
- localVersion,
- Upgrade.Status.INACTIVE);
- if (!state.compareAndSet(reset, inactive)) {
- throw new IllegalStateException("Concurrent upgrade modification");
- } else {
- currentState.set(inactive);
- }
- }
- }
+ // Set the upgrade status to INACTIVE.
+ Upgrade inactive = new Upgrade(
+ localVersion,
+ localVersion,
+ Upgrade.Status.INACTIVE);
+ changeState(reset, inactive);
}
/**
@@ -380,9 +367,10 @@
* @param event the cluster event
*/
protected void handleClusterEvent(ClusterEvent event) {
+ checkPermission(CLUSTER_EVENT);
// If an instance was deactivated, check whether we need to roll back the upgrade.
if (event.type() == ClusterEvent.Type.INSTANCE_DEACTIVATED) {
- Upgrade upgrade = state.get();
+ Upgrade upgrade = getState();
if (upgrade.status().upgraded()) {
// Get the upgraded subset of the cluster and check whether the down node is a member
// of the upgraded subset. If so, roll back the upgrade to tolerate the failure.
@@ -403,6 +391,7 @@
* @param event the upgrade value event
*/
protected void handleUpgradeEvent(AtomicValueEvent<Upgrade> event) {
+ checkPermission(UPGRADE_EVENT);
currentState.set(event.newValue());
switch (event.newValue().status()) {
case INITIALIZED:
diff --git a/core/security/src/main/java/org/onosproject/security/impl/DefaultPolicyBuilder.java b/core/security/src/main/java/org/onosproject/security/impl/DefaultPolicyBuilder.java
index 8877e56..1879997 100644
--- a/core/security/src/main/java/org/onosproject/security/impl/DefaultPolicyBuilder.java
+++ b/core/security/src/main/java/org/onosproject/security/impl/DefaultPolicyBuilder.java
@@ -20,11 +20,11 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import org.onosproject.cluster.ClusterAdminService;
+import org.onosproject.cluster.ClusterMetadataAdminService;
import org.onosproject.cluster.ClusterMetadataService;
import org.onosproject.cluster.ClusterService;
-import org.onosproject.cluster.ClusterMetadataAdminService;
-import org.onosproject.cluster.LeadershipService;
import org.onosproject.cluster.LeadershipAdminService;
+import org.onosproject.cluster.LeadershipService;
import org.onosproject.codec.CodecService;
import org.onosproject.event.EventDeliveryService;
import org.onosproject.mastership.MastershipTermService;
@@ -75,6 +75,8 @@
import org.onosproject.store.service.StorageAdminService;
import org.onosproject.store.service.StorageService;
import org.onosproject.ui.UiExtensionService;
+import org.onosproject.upgrade.UpgradeAdminService;
+import org.onosproject.upgrade.UpgradeService;
import org.osgi.framework.ServicePermission;
import org.osgi.framework.AdminPermission;
import org.osgi.framework.AdaptPermission;
@@ -202,7 +204,6 @@
permSet.add(new ServicePermission(RegionAdminService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(PartitionAdminService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(StorageAdminService.class.getName(), ServicePermission.GET));
-
permSet.add(new ServicePermission(ApplicationService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(ComponentConfigService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(ClusterMetadataService.class.getName(), ServicePermission.GET));
@@ -247,7 +248,8 @@
permSet.add(new ServicePermission(LogicalClockService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(StorageService.class.getName(), ServicePermission.GET));
permSet.add(new ServicePermission(UiExtensionService.class.getName(), ServicePermission.GET));
-
+ permSet.add(new ServicePermission(UpgradeService.class.getName(), ServicePermission.GET));
+ permSet.add(new ServicePermission(UpgradeAdminService.class.getName(), ServicePermission.GET));
return permSet;
}
@@ -367,6 +369,12 @@
PartitionService.class.getName()));
serviceDirectory.put(CLOCK_WRITE, ImmutableSet.of(
LogicalClockService.class.getName()));
+ serviceDirectory.put(UPGRADE_READ, ImmutableSet.of(
+ UpgradeService.class.getName(), UpgradeAdminService.class.getName()));
+ serviceDirectory.put(UPGRADE_WRITE, ImmutableSet.of(
+ UpgradeAdminService.class.getName()));
+ serviceDirectory.put(UPGRADE_EVENT, ImmutableSet.of(
+ UpgradeService.class.getName(), UpgradeAdminService.class.getName()));
return serviceDirectory;
}