[Pki-devel] [PATCH] 0159..0161 Fix config param removal in profile modification
Fraser Tweedale
ftweedal at redhat.com
Thu Apr 20 01:48:12 UTC 2017
I have created a gerrit review for this patchset:
https://review.gerrithub.io/#/c/357607/
Thanks,
Fraser
On Tue, Feb 07, 2017 at 09:39:52PM +1000, Fraser Tweedale wrote:
> 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
> 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
>
> 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
>
> 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
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
More information about the Pki-devel
mailing list