Skip to content

Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting #11567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: 4.20
Choose a base branch
from
+40 −2
Open

Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting #11567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe996d6
Check backup framework enabled config in zone scope first (before che…
sureshanaparti Sep 3, 2025
4af74e6
Update backup.framework.enabled config to Global scope
sureshanaparti Sep 9, 2025
1543511
Revert "Update backup.framework.enabled config to Global scope"
sureshanaparti Sep 23, 2025
878aa4c
Check the config key and name exists in its scope (without scopie id)
sureshanaparti Sep 24, 2025
2dffcda
code improvements
sureshanaparti Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,9 @@ public String getActualValue(ResourceDetail resourceDetail) {
}
return resourceDetail.getValue();
}

public boolean doesKeyValuePairExist(String key, String value) {
List<R> details = findDetails(key, value, null);
return CollectionUtils.isNotEmpty(details);
}
}
6 changes: 6 additions & 0 deletions framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,10 @@ protected T valueOf(String value) {
}
}

public boolean hasValueInScope(String value) {
if (value != null && s_depot != null) {
return s_depot.doesConfigKeyAndValueExistInScope(_name, value, _scope);
}
return false;
}
}
6 changes: 6 additions & 0 deletions ...work/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,10 @@ public interface ScopedConfigStorage {
default String getConfigValue(long id, ConfigKey<?> key) {
return getConfigValue(id, key.key());
}

boolean doesKeyValuePairExist(String key, String value);

default boolean doesConfigKeyAndValueExist(String key, String value) {
return doesKeyValuePairExist(key, value);
}
}
21 changes: 21 additions & 0 deletions ...ork/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,27 @@ protected String getConfigStringValueInternal(Ternary<String, ConfigKey.Scope, L
return null;
}

public boolean doesConfigKeyAndValueExistInScope(String key, String value, ConfigKey.Scope scope) {
if (!ConfigKey.Scope.Global.equals(scope)) {
ScopedConfigStorage scopedConfigStorage = null;
for (ScopedConfigStorage storage : _scopedStorages) {
if (storage.getScope() == scope) {
scopedConfigStorage = storage;
}
}
if (scopedConfigStorage == null) {
logger.warn("Unable to check existence of config key {} and value {} in scope: {}, couldn't find config storage for this scope", key, value, scope);
return false;
}
return scopedConfigStorage.doesConfigKeyAndValueExist(key, value);
}
ConfigurationVO configurationVO = _configDao.findByName(key);
if (configurationVO != null) {
Comment on lines +313 to +314
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Global scope value() is good enough, right?
why do we need this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abh1sar some earlier config keys are not part of config framework, so these are fetched directly from the db. it's the same with the current method getConfigStringValueInternal() as well.

return configurationVO.getValue().equalsIgnoreCase(value);
}
return false;
}

protected Ternary<String, ConfigKey.Scope, Long> getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) {
return new Ternary<>(key, scope, scopeId);
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
}

public boolean isDisabled(final Long zoneId) {
return !(BackupFrameworkEnabled.value() && BackupFrameworkEnabled.valueIn(zoneId));
return !(BackupFrameworkEnabled.valueIn(zoneId));
}

private void validateForZone(final Long zoneId) {
Expand Down Expand Up @@ -980,7 +980,7 @@ public BackupProvider getBackupProvider(final String name) {
@Override
public List<Class<?>> getCommands() {
final List<Class<?>> cmdList = new ArrayList<Class<?>>();
if (!BackupFrameworkEnabled.value()) {
if (!BackupFrameworkEnabled.value() && !BackupFrameworkEnabled.hasValueInScope(Boolean.TRUE.toString())) {
Copy link
Contributor

@shwstppr shwstppr Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not restrict/allow API access based on the zone config value. It might break UI/clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shwstppr it doesn't restrict API access. it allows API access if it is enabled in global or in any zone (not mandatory to enable in global scope). previously, enabling at global level is mandatory for API access.

return cmdList;
}

Expand Down
Loading