[Pki-devel] [PATCH] 0159..0161 Fix config param removal in profile modification
Fraser Tweedale
ftweedal at redhat.com
Tue Feb 7 11:39:52 UTC 2017
Please review the attached patches which fix
https://fedorahosted.org/pki/ticket/2588, a bug in profile
modification where config params can only be added or changed, but
not removed.
Thanks,
Fraser
-------------- next part --------------
From 0a86f63cfe2d5391befe401541e9dcc0dae6ce29 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Tue, 7 Feb 2017 17:27:06 +1000
Subject: [PATCH 159/161] LDAPProfileSubsystem: avoid duplicating logic in
superclass
Part of: https://fedorahosted.org/pki/ticket/2588
---
.../cmscore/profile/AbstractProfileSubsystem.java | 7 +++-
.../cmscore/profile/LDAPProfileSubsystem.java | 43 ++++------------------
2 files changed, 13 insertions(+), 37 deletions(-)
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
index 116b8e2026e80b012fb87647fd8924b567194fa3..2a209ad5b2656d65db57d36b7ecb2745527ab081 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java
@@ -121,7 +121,7 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
/**
* Commits a profile.
*/
- public void commitProfile(String id)
+ public synchronized void commitProfile(String id)
throws EProfileException {
IConfigStore cs = mProfiles.get(id).getConfigStore();
@@ -157,6 +157,11 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem {
// finally commit the configStore
//
+ commitConfigStore(id, cs);
+ }
+
+ protected void commitConfigStore(String id, IConfigStore cs)
+ throws EProfileException {
try {
cs.commit(false);
} catch (EBaseException e) {
diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
index fff8ead3f2088aedaf5856c308dd33be90af7779..bce675e7bf993d97a086fb830e34d50000c4f396 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java
@@ -303,43 +303,14 @@ public class LDAPProfileSubsystem
readProfile(entry);
}
+ /**
+ * Commit the configStore and track the resulting
+ * entryUSN and (in case of add) the nsUniqueId
+ */
@Override
- public synchronized void commitProfile(String id) throws EProfileException {
- LDAPConfigStore cs = (LDAPConfigStore) mProfiles.get(id).getConfigStore();
-
- // first create a *new* profile object from the configStore
- // and initialise it with the updated configStore
- //
- IPluginRegistry registry = (IPluginRegistry)
- CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY);
- String classId = mProfileClassIds.get(id);
- IPluginInfo info = registry.getPluginInfo("profile", classId);
- String className = info.getClassName();
- IProfile newProfile = null;
- try {
- newProfile = (IProfile) Class.forName(className).newInstance();
- } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
- throw new EProfileException("Could not instantiate class '"
- + classId + "' for profile '" + id + "': " + e);
- }
- newProfile.setId(id);
- try {
- newProfile.init(this, cs);
- } catch (EBaseException e) {
- throw new EProfileException(
- "Failed to initialise profile '" + id + "': " + e);
- }
-
- // next replace the existing profile with the new profile;
- // this is to avoid any intermediate state where the profile
- // is not fully initialised with its inputs, outputs and
- // policy objects.
- //
- mProfiles.put(id, newProfile);
-
- // finally commit the configStore and track the resulting
- // entryUSN and (in case of add) the nsUniqueId
- //
+ protected void commitConfigStore(String id, IConfigStore configStore)
+ throws EProfileException {
+ LDAPConfigStore cs = (LDAPConfigStore) configStore;
try {
String[] attrs = {"entryUSN", "nsUniqueId"};
LDAPEntry entry = cs.commitReturn(false, attrs);
--
2.9.3
-------------- next part --------------
From ca09f58f4a953fb8d40898a1924f236bba42fa29 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Tue, 7 Feb 2017 17:39:33 +1000
Subject: [PATCH 160/161] ISourceConfigStore: add clear() method to interface
The SourceConfigStore load() method does not clear the config store,
but this might be necessary to avoid stale data if wanting to
perform a complete replacement of the data (e.g. reload from file).
We should not change the behaviour of load() in case some code is
relying on the current behaviour, so add the clear() method to the
interface.
Part of: https://fedorahosted.org/pki/ticket/2588
---
base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java | 5 +++++
.../cmscore/src/com/netscape/cmscore/base/PropConfigStore.java | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java b/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java
index 42637c258258472d8469fd8c691a826e3bcb1b3e..8eb86c2ba5cfd1522f52b98a676d1a509424c270 100644
--- a/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java
+++ b/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java
@@ -63,6 +63,11 @@ public interface ISourceConfigStore extends Serializable {
public Enumeration<String> keys();
/**
+ * Clear the config store.
+ */
+ public void clear();
+
+ /**
* Reads a config store from an input stream.
*
* @param in input stream where the properties are located
diff --git a/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java b/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java
index cc16e247d01428f958d0d397ff95127fcb8d2f45..acf28441337b76628d47dc58351a0233568397d8 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java
@@ -223,6 +223,10 @@ public class PropConfigStore implements IConfigStore, Cloneable {
}
}
+ public synchronized void clear() {
+ mSource.clear();
+ }
+
/**
* Reads a config store from an input stream.
*
--
2.9.3
-------------- next part --------------
From 87ceb6f2e0ad93359f6ca94a08a8cb7f2bbfeaaa Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at redhat.com>
Date: Tue, 7 Feb 2017 21:12:08 +1000
Subject: [PATCH 161/161] ProfileService: clear profile attributes when
modifying
When modifying a profile, attributes are not cleared. Attributes
that were removed in the updated profile configuration are not
actually removed.
When updating a profile via PUT /ca/rest/profiles/{id}/raw, clear
the config store before loading the new configuration.
Fixes: https://fedorahosted.org/pki/ticket/2588
---
base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
index 41d009b9d38003f054f45c2dd8070de8f46065f3..26e86c51d388d83fc7070b355505f011426350c0 100644
--- a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
+++ b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java
@@ -738,6 +738,7 @@ public class ProfileService extends PKIService implements ProfileResource {
}
// no error thrown, so commit updated profile config
+ profile.getConfigStore().clear();
profile.getConfigStore().load(new ByteArrayInputStream(data));
ps.disableProfile(profileId);
ps.commitProfile(profileId);
--
2.9.3
More information about the Pki-devel
mailing list