Re: [Spacewalk-devel] [PATCH] SUSE Studio API will stop working via unencrypted HTTP

2014-02-05 Thread Johannes Renner

On 01/22/2014 12:18 PM, Johannes Renner wrote:

Hello,

The SUSE Studio API will no longer process requests via unencrypted HTTP after
Jan 2014, see here:

http://blog.susestudio.com/2013/12/coming-soon-https-only-on-suse-studio.html

Therefore attached is a patch to change the default endpoint to use SSL.


Following up on this issue, here is two more related patches: One is to update
the specfile in spec-tree in order to build a more recent version of the client
library and another one to make the spacewalk code compile against that version.

Note that the integration part will work with the old lib as well since we don't
use the default endpoint that comes with the library, but we rather set it from
within spacewalk. I would still recommend to update the library anyways.

Thanks and regards,
Johannes

--
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From e64a4b53a4d3bbd04af4cbe226753223d4e2cdba Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 5 Feb 2014 11:55:17 +0100
Subject: [PATCH 1/2] Update susestudio-java-client spec file for building
 version 0.1.4

---
 spec-tree/susestudio-java-client/susestudio-java-client.spec | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/spec-tree/susestudio-java-client/susestudio-java-client.spec b/spec-tree/susestudio-java-client/susestudio-java-client.spec
index 1965cb0..99ca9de 100644
--- a/spec-tree/susestudio-java-client/susestudio-java-client.spec
+++ b/spec-tree/susestudio-java-client/susestudio-java-client.spec
@@ -1,7 +1,7 @@
 #
 # spec file for package susestudio-java-client
 #
-# Copyright (c) 2012 SUSE LINUX Products GmbH, Nuremberg, Germany.
+# Copyright (c) 2014 SUSE LINUX Products GmbH, Nuremberg, Germany.
 #
 # All modifications and additions to the file contributed by third parties
 # remain the property of their copyright owners, unless otherwise agreed
@@ -12,14 +12,11 @@
 # license that conforms to the Open Source Definition (Version 1.9)
 # published by the Open Source Initiative.
 
-# Please submit bugfixes or comments via http://bugs.opensuse.org/
-#
-
 %define third_party_jars simple-xml
 
 Name:   susestudio-java-client
 Summary:Java client library for SUSE Studio
-Version:0.1.2
+Version:0.1.4
 Release:2%{?dist}
 License:MIT
 Group:  Development/Libraries/Java
@@ -46,7 +43,7 @@ rm lib/*.jar
 build-jar-repository -p lib/ %third_party_jars
 
 %build
-ant -Dant.build.javac.source=1.5 -Dant.build.javac.target=1.5
+ant -Dant.build.javac.source=1.5 -Dant.build.javac.target=1.5 dist-jar
 
 %install
 install -d -m 0755 $RPM_BUILD_ROOT%{_javadir}
-- 
1.8.5.3

From 47fbff436a823da8c629f37df39bff7bd690f222 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 5 Feb 2014 11:56:53 +0100
Subject: [PATCH 2/2] Patch code to build against susestudio-java-client
 version 0.1.4

---
 java/code/src/com/redhat/rhn/domain/image/Image.java| 6 +++---
 .../com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java| 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/image/Image.java b/java/code/src/com/redhat/rhn/domain/image/Image.java
index a3ad87c..47997d9 100644
--- a/java/code/src/com/redhat/rhn/domain/image/Image.java
+++ b/java/code/src/com/redhat/rhn/domain/image/Image.java
@@ -26,7 +26,7 @@ public class Image extends BaseDto implements ComparableImage {
 private String name;
 private String version;
 private String arch;
-private String imageSize;
+private int imageSize;
 private String imageType;
 private String downloadUrl;
 private String editUrl;
@@ -100,7 +100,7 @@ public class Image extends BaseDto implements ComparableImage {
  * Get the image size.
  * @return image size
  */
-public String getImageSize() {
+public int getImageSize() {
 return this.imageSize;
 }
 
@@ -108,7 +108,7 @@ public class Image extends BaseDto implements ComparableImage {
  * Set the image size.
  * @param imageSizeIn image size
  */
-public void setImageSize(String imageSizeIn) {
+public void setImageSize(int imageSizeIn) {
 this.imageSize = imageSizeIn;
 }
 
diff --git a/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java b/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
index dea77e7..1a42d18 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
@@ -15,7 +15,6 @@
 
 package com.redhat.rhn.frontend.action.renderers;
 
-import java.io.IOException;
 import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -34,6 +33,7 @@ import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.listview.PageControl;
 import

Re: [Spacewalk-devel] [PATCH] Allow null as createdBy and lastModifiedBy for custom data values

2014-01-23 Thread Johannes Renner
On 01/21/2014 06:10 PM, Johannes Renner wrote:
 On 01/21/2014 03:38 PM, Jan Dobes wrote:
 Hello Johannes,

 I'm not sure how to reproduce this. I tried to create user, create custom 
 key, assign value to system, delete user, changing values...
 Except some ISEs in WebUI, indexation works fine.
 Even if tried to insert directly to db on clean PostgreSQL Spacewalk nightly:

 insert into rhnServerCustomDataValue values (11, 6, 'qwerty', null, 
 null, current_timestamp, current_timestamp); -- with certain key and system, 
 both values null

 It properly indexed, after changing value too. I tested with searching by 
 Custom Info.
 
 Hello,
 
 I can reproduce it using your insert statement (with existing server and key 
 ids of course).
 This was with a previous version of our product, though, since this was 
 currently the only
 server around not having that patch installed. I can imagine that in the 
 meantime the issue
 has been fixed by a library update as well?
 
 I can see that ibatis has been updated (and the exception came from there, 
 see my attachment
 for the complete stacktrace):
 
 - ibatis-2.3.0.677.jar - mybatis-3.2.3.jar
 
 Note that in order to see that exception in the logs I had to set:
 
 log4j.logger.org.quartz=INFO,SearchAppender 
 (/usr/share/rhn/search/classes/log4j.properties)
 
 If it has been fixed by a library update you don't have to commit the patch 
 of course.
 Will try again to reproduce it tomorrow with a server running recent mybatis.

Ok, I tried again with a recent version and you are right: I can't reproduce it 
there
either. So maybe consider this patch to be relevant for older installations 
only.

Thank you for your efforts anyways,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] SUSE Studio API will stop working via unencrypted HTTP

2014-01-22 Thread Johannes Renner
Hello,

The SUSE Studio API will no longer process requests via unencrypted HTTP after
Jan 2014, see here:

http://blog.susestudio.com/2013/12/coming-soon-https-only-on-suse-studio.html

Therefore attached is a patch to change the default endpoint to use SSL.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From f523f1d0188c4094888513026478435db169ce3c Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 22 Jan 2014 10:41:08 +0100
Subject: [PATCH] SUSE Studio API will stop working via unencrypted HTTP

---
 .../com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
index 66efc89..72395f3 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
@@ -40,7 +40,7 @@ public class UserCredentialsEditAction extends RhnAction {
 private static final String PARAM_USER = studio_user;
 private static final String PARAM_KEY = studio_key;
 private static final String PARAM_URL = studio_url;
-private static final String DEFAULT_URL = http://susestudio.com;;
+private static final String DEFAULT_URL = https://susestudio.com;;
 
 /** {@inheritDoc} */
 @Override
-- 
1.8.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Allow null as createdBy and lastModifiedBy for custom data values

2014-01-17 Thread Johannes Renner
Hello,

I found another issue with the custom data values: It's a known and fixed bug 
(889633)
that the indexing job will crash when initially *not* setting the 
lastModifiedBy column.
This column can still get null whenever a creator or modifier is deleted since 
there is
a ON DELETE SET NULL on these columns. We therefore have to allow null there, 
please see
and apply my attached patch.

This can also be problematic when there is values that were created before 
installing
the fix for 889633. These will have null as lastModifiedBy as long as they have 
never
been edited. Even after installing that fix, the old values will make the 
indexer crash.
The attached patch will also fix such cases.

Otherwise what we see is the same exception as with the above bug and no custom 
data
values are being indexed:

INFO   | jvm 1| 2014/01/16 08:47:51 | --- Cause: 
java.lang.RuntimeException: Error setting
property 'setLastModifiedBy' of 
'com.redhat.satellite.search.db.models.ServerCustomInfo@75807580'.
Cause: java.lang.IllegalArgumentException]
INFO   | jvm 1| 2014/01/16 08:47:51 |   at
com.redhat.satellite.search.scheduler.tasks.GenericIndexTask.execute(GenericIndexTask.java:91)

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 910d1db851a485ec565f7e59f6b1d0d89c29 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 17 Jan 2014 12:01:45 +0100
Subject: [PATCH] Allow null as createdBy and lastModifiedBy

These can get null when a creator or modifier is being deleted
(ON DELETE SET NULL).
---
 .../redhat/satellite/search/db/models/ServerCustomInfo.java  | 12 ++--
 .../search/scheduler/tasks/IndexServerCustomInfoTask.java|  8 ++--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
index 3b0b53f..084898f 100644
--- a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
+++ b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
@@ -24,8 +24,8 @@ public class ServerCustomInfo extends GenericRecord {
 private long serverId;
 private String label;
 private String value;
-private long createdBy;
-private long lastModifiedBy;
+private Long createdBy;
+private Long lastModifiedBy;
 private String created;
 private String modified;
 
@@ -121,25 +121,25 @@ public class ServerCustomInfo extends GenericRecord {
 /**
  * @return the createdBy
  */
-public long getCreatedBy() {
+public Long getCreatedBy() {
 return createdBy;
 }
 /**
  * @param createdByIn the createdBy to set
  */
-public void setCreatedBy(long createdByIn) {
+public void setCreatedBy(Long createdByIn) {
 this.createdBy = createdByIn;
 }
 /**
  * @return the lastModifiedBy
  */
-public long getLastModifiedBy() {
+public Long getLastModifiedBy() {
 return lastModifiedBy;
 }
 /**
  * @param lastModifiedByIn the lastModifiedBy to set
  */
-public void setLastModifiedBy(long lastModifiedByIn) {
+public void setLastModifiedBy(Long lastModifiedByIn) {
 this.lastModifiedBy = lastModifiedByIn;
 }
 
diff --git a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
index 412b598..57d109a 100644
--- a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
+++ b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
@@ -97,8 +97,12 @@ public class IndexServerCustomInfoTask extends GenericIndexTask {
 attrs.put(serverId, new Long(scInfo.getServerId()).toString());
 attrs.put(value, scInfo.getValue());
 attrs.put(label, scInfo.getLabel());
-attrs.put(createdBy, new Long(scInfo.getCreatedBy()).toString());
-attrs.put(lastModifiedBy, new Long(scInfo.getLastModifiedBy()).toString());
+if (scInfo.getCreatedBy() != null) {
+attrs.put(createdBy, scInfo.getCreatedBy().toString());
+}
+if (scInfo.getLastModifiedBy() != null) {
+attrs.put(lastModifiedBy, scInfo.getLastModifiedBy().toString());
+}
 attrs.put(created, scInfo.getCreated());
 attrs.put(modified, scInfo.getModified());
 return attrs;
-- 
1.8.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password strength meter to spacewalk

2014-01-08 Thread Johannes Renner
On 01/08/2014 02:34 PM, Michael Mraka wrote:
 Maximilian Meister wrote:
 % I see. Then it should keep the original name so we can easily figure out
 % where it came from and replace it with newer version in the future.
 % 
 % Hi Michael,
 % 
 % the original name would be pwstrength.js (in 0.5.0). We decided to
 % use the spacewalk- prefix to distinguish it for all JavaScript
 % related to the password strength meter in one single file.
 % 
 % Which version of jquery.pwstrength.bootstrap was it? It doesn't match to
 % any pwstrength-bootstrap-1.0.X.js.
 % 
 % 0.5.0
 
 As it's new feature in spacewalk I'd vote for using current latest
 version of pwstrength-bootstrap.

I would also vote for using the latest available version if possible, ideally
making it even easy to upgrade the upstream code to newer future versions.

 % Is it possible to keep original pwstrength-bootstrap*.js unmodified and
 % put modification to the separate .js (call modified functions from the
 % page and call original functions from them)?
 % 
 % To keep the pwstrength.js library itself separate for easier update
 % on a new version makes sense but has a few issues.
 % I needed to change the library itself to make it work and look good
 % for spacewalk.
 % I had to change/add some generated html output (html tags, add css
 % classes), some logic and css selectors.
 % These are changes only make sense for spacewalk specific look and
 % functionality.
 
 I understand it. In such cases where we need to modify upstream sources
 we put upstream package spec to spec-tree/ and create patches to it.
 This let's us easily keep our modifications and re-apply it on new
 upstream versions whenever wee need.
 See e.g. spec-tree/stringtree-json in spacewalk.git.

Sounds like a good workaround to me for solving the problem, at least if we
can't simply outsource our modifications into a separate javascript file.

 % Furthermore in 1.0.2 the pwstrength.js is now separated into 4
 % different js files.
 
 Isn't
 https://github.com/ablanco/jquery.pwstrength.bootstrap/blob/master/dist/pwstrength-bootstrap-1.0.2.js
 all we need to distribute?

I would guess so, this seems to be just a bundle containing contents of all
those 4 source files, right?

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password strength meter to spacewalk

2014-01-07 Thread Johannes Renner
On 12/20/2013 05:15 PM, R P Herrold wrote:
 On Fri, 20 Dec 2013, Maximilian Meister wrote:
 
 this patch would add a bootstrapified password strength 
 meter to all pages where user details are being created or 
 edited (create the initial admin user, create/edit normal 
 users and create organization). There is also a tick icon on 
 the side of the password input fields, which checks if the 
 value in the desired password field will be accepted by the 
 server and if the value in the confirm password field 
 matches the value in the desired password field.
 
 This seems to pull in new dependencies, doesn't it?  It is not 
 really clear to me that doing passwd strength testing here, 
 'one off' per package makes sense, rather than having a 
 general interface to query a facility that many packages 
 beyond just spacewalk could use

Maximilian's patch is in fact based on a third party plugin for Twitter 
Bootstrap:

https://github.com/ablanco/jquery.pwstrength.bootstrap

I agree that in fact it would be good practice to package the plugin javascript
file, so it can be used beyond spacewalk as you are suggesting. In case 
Maximilian
has upstreamed all the changes he made I would propose him to do that.

But please also note that Spacewalk included libraries like prototype-1.6.0 and
scriptaculous since 2008, and these were never put into separate packages so 
far!
Now these were exchanged for bootstrap, jquery and less, so yes: I agree that 
all
those libs should ideally be separate packages, but it's a general problem that 
is
not specific to this password strength meter proposal.

 Also, how would this interact with non-local 
 authentication interfaces such as LDAP?

Just as Silvio wrote, it won't. It really is only for giving immediate feedback 
to
users about the strength of a proposed password while typing.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Fix NPE when uploading kickstart profile

2013-12-18 Thread Johannes Renner
Hey,

we found that one of your recent bugfixes (see 009fab0) causes a NPE when trying
to upload a kickstart profile with Virtualization Type set to None.

Consider to apply the attached patch after review to fix the problem.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From dbe021e76741e7f8a503ad37021e78deeeb13c3f Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 18 Dec 2013 11:32:26 +0100
Subject: [PATCH] Fix NPE when uploading kickstart profile with virt type none

---
 .../frontend/action/kickstart/KickstartDetailsEditAction.java  | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartDetailsEditAction.java b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartDetailsEditAction.java
index 850dba5..0dfda3d 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartDetailsEditAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartDetailsEditAction.java
@@ -290,9 +290,11 @@ public class KickstartDetailsEditAction extends BaseKickstartEditAction {
  */
 public static void processCobblerFormValues(KickstartData ksdata, DynaActionForm form,
 User user) throws ValidatorException {
-int virtMemory = (Integer) form.get(VIRT_MEMORY);
-if (ksdata.isRhel7OrGreater()  virtMemory  768) {
-ValidatorException.raiseException(kickstart.cobbler.profile.notenoughmemory);
+if (KickstartDetailsEditAction.canSaveVirtOptions(ksdata, form)) {
+int virtMemory = (Integer) form.get(VIRT_MEMORY);
+if (ksdata.isRhel7OrGreater()  virtMemory  768) {
+ValidatorException.raiseException(kickstart.cobbler.profile.notenoughmemory);
+}
 }
 
 CobblerProfileEditCommand cmd = new CobblerProfileEditCommand(ksdata, user);
@@ -309,7 +311,7 @@ public class KickstartDetailsEditAction extends BaseKickstartEditAction {
 }
 
 if (KickstartDetailsEditAction.canSaveVirtOptions(ksdata, form)) {
-prof.setVirtRam(virtMemory);
+prof.setVirtRam((Integer) form.get(VIRT_MEMORY));
 prof.setVirtCpus((Integer) form.get(VIRT_CPU));
 prof.setVirtFileSize((Integer) form.get(VIRT_DISK_SIZE));
 prof.setVirtBridge(form.getString(VIRT_BRIDGE));
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Fix the Java package of DeleteGroupAction class

2013-12-18 Thread Johannes Renner
Hello,

here is another micropatch fixing up a recent commit in master. The Java
package needs to correspond with the filesystem path in any case.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 8e0a0192c54255e50a760887e1349d55492ab813 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 18 Dec 2013 14:57:00 +0100
Subject: [PATCH] Fix the java package of DeleteGroupAction class

---
 .../src/com/redhat/rhn/frontend/action/groups/DeleteGroupAction.java| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/groups/DeleteGroupAction.java b/java/code/src/com/redhat/rhn/frontend/action/groups/DeleteGroupAction.java
index 1bbc4ad..41161c4 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/groups/DeleteGroupAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/groups/DeleteGroupAction.java
@@ -12,7 +12,7 @@
  * granted to use or replicate Red Hat trademarks that are incorporated
  * in this software or its documentation.
  */
-package com.redhat.rhn.frontend.action.token;
+package com.redhat.rhn.frontend.action.groups;
 
 import com.redhat.rhn.domain.server.ManagedServerGroup;
 import com.redhat.rhn.frontend.struts.RequestContext;
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] License of spacewalk-branding needs to be changed

2013-11-21 Thread Johannes Renner
Hello,

The license of the spacewalk-branding package needs to be changed since from 
now on
the following css frameworks, fonts and icons will be included with the package:

- Twitter Bootstrap (Apache-2.0)
- Font Awesome (MIT and OFL-1.1)
- Roboto Font (Apache-2.0)

See this bug for tracking the issue:

https://bugzilla.redhat.com/show_bug.cgi?id=1033062

Please see also the attached patch changing the license using the spdx specs 
[1].

Regards,
Johannes

[1] http://spdx.org/licenses/

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 633c050f3a068aa8a52a217c5f6ea55cd15a3937 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 21 Nov 2013 15:00:27 +0100
Subject: [PATCH] Fix the License of spacewalk-branding package

---
 branding/spacewalk-branding.spec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/branding/spacewalk-branding.spec b/branding/spacewalk-branding.spec
index 5194fdd..d4646f1 100644
--- a/branding/spacewalk-branding.spec
+++ b/branding/spacewalk-branding.spec
@@ -28,7 +28,7 @@ Release:1%{?dist}
 Summary:Spacewalk branding data
 
 Group:  Applications/Internet
-License:GPLv2
+License:Apache-2.0 and GPL-2.0 and MIT and OFL-1.1
 URL:https://fedorahosted.org/spacewalk/
 Source0:https://fedorahosted.org/releases/s/p/spacewalk/%{name}-%{version}.tar.gz
 BuildRoot:  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Fix Search for Custom Info Values + NPE

2013-11-12 Thread Johannes Renner
Hey,

attached is a patch to fix the advanced search for custom info values. The 
indexing seems to work
fine, it's only the removal of old index records that is failing. And AFAICS it 
is failing for
custom info values only while it seems to work fine for e.g. system 
name/description.

The issue is when a custom value is changed and you do a search for the old 
value: you will still
get the respective system as a result. This should not be the case of course, 
try to reproduce it!
Reason seems to be that certain methods in lucene (or at least that old version 
that is used) don't
really work as expected with the combined identifier that we use for custom 
info values. Especially
reader.docFreq(term) always returns 0 in IndexManager.addUniqueToIndex(), for 
any reason. Therefore
we will never remove any old index files when trying to add one uniquely. I 
tried several fixes,
but the only thing that I found to be working was overwriting getId() to return 
the concatenated
IDs as a long, please see my patch.

The second patch is related to the same feature. It prevents an NPE when trying 
to delete a custom
info value that has not been persisted yet. Try it out by creating a new custom 
key value and
before clicking on Update Key, do delete value above.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From a61776d4e453cdc7462ee95a0b9a3077f0fe886d Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 12 Nov 2013 16:26:31 +0100
Subject: [PATCH 1/2] Fix custom info value index removal in advanced search

---
 .../redhat/satellite/search/db/models/ServerCustomInfo.java| 10 +-
 .../search/scheduler/tasks/IndexServerCustomInfoTask.java  |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
index 8626289..3b0b53f 100644
--- a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
+++ b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/db/models/ServerCustomInfo.java
@@ -33,17 +33,17 @@ public class ServerCustomInfo extends GenericRecord {
  *
  * @param serverIdIn server id
  * @param keyIdIn key id
- * @return string based on server id and key id
+ * @return long based on server id and key id
  */
-public static String makeUniqId(long serverIdIn, long keyIdIn) {
-return serverIdIn + - + keyIdIn;
+public static long makeUniqId(long serverIdIn, long keyIdIn) {
+return new Long( + serverIdIn + keyIdIn);
 }
 
 /**
  *
- * @return uniqId
+ * @return unique id for a server/custom info key pair
  */
-public String getUniqId() {
+public long getId() {
 return makeUniqId(serverId, keyId);
 }
 /**
diff --git a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
index f9236c2..412b598 100644
--- a/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
+++ b/search-server/spacewalk-search/src/java/com/redhat/satellite/search/scheduler/tasks/IndexServerCustomInfoTask.java
@@ -62,7 +62,7 @@ public class IndexServerCustomInfoTask extends GenericIndexTask {
 }
 idSet = new HashSet();
 for (ServerCustomInfo scInfo : ids) {
-idSet.add(scInfo.getUniqId());
+idSet.add(new Long(scInfo.getId()).toString());
 }
 uniqField = getUniqueFieldId();
 indexName = getIndexName();
@@ -93,7 +93,7 @@ public class IndexServerCustomInfoTask extends GenericIndexTask {
 throws ClassCastException {
 ServerCustomInfo scInfo = (ServerCustomInfo)data;
 MapString, String attrs = new HashMapString, String();
-attrs.put(uniqId, scInfo.getUniqId());
+attrs.put(id, new Long(scInfo.getId()).toString());
 attrs.put(serverId, new Long(scInfo.getServerId()).toString());
 attrs.put(value, scInfo.getValue());
 attrs.put(label, scInfo.getLabel());
@@ -156,7 +156,7 @@ public class IndexServerCustomInfoTask extends GenericIndexTask {
  * {@inheritDoc}
  */
 public String getUniqueFieldId() {
-return uniqId;
+return id;
 }
 /**
  * {@inheritDoc}
-- 
1.8.1.4

From b0c4c5f7ab2570e58403a87a24ddae77566bbee6 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 12 Nov 2013 16:30:10 +0100
Subject: [PATCH 2/2] Fix ISE when deleting a non persistent custom info value

---
 .../action/systems/sdc/DeleteCustomDataAction.java | 28

Re: [Spacewalk-devel] Less compiler package to use

2013-10-24 Thread Johannes Renner
On 10/22/2013 02:07 PM, Tomáš Kašpárek wrote:
 I've noticed that some icons (e.g. in legend text on the left) are missing, 
 also search button is
 missing some picture or text. On pages like 
 /rhn/channels/ChannelDetail.do?cid=101 (Channels - pick
 one from displayed channels) old update button is used (not the green 
 bootstrap one). I've found
 some small issues but I think they'll be fixed as it's still work in progress.

This sounds like you were missing the new fonts folder we created. We pushed 
some recent changes,
so if you want you can see if at least missing icons are fixed. I added the 
fonts folder with:

- 6318f88adfa18558b21363f337f856585adf5bf3

Note that the license of this package will probably have to be changed since we 
added font-awesome,
twitter bootstrap less files, as well as the Roboto font.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Less compiler package to use

2013-10-22 Thread Johannes Renner
On 10/22/2013 01:05 PM, Tomáš Kašpárek wrote:
 Hello Johannes,
 
 I've discovered issue in 12ce0476ba547fade1cbe3819e77fd560de7fdb0 which 
 causes RPM build errors
 during build time.
 
 RPM build errors:
 File not found:
 /root/rpmbuild/BUILDROOT/spacewalk-branding-2.1.5-1.git.233.e7a7524.el6.x86_64/var/www/html/css
 File not found by glob:
 /root/rpmbuild/BUILDROOT/spacewalk-branding-2.1.5-1.git.233.e7a7524.el6.x86_64/var/www/html/css/*
 
 cp -pR css %{buildroot}/%{_var}/www/html/ copied whole css directory to 
 /%{_var}/www/html efectively
 making it /%{_var}/www/html/css.
 
 cp -p css/*.css %{buildroot}/%{_var}/www/html/ takes matched files and copies 
 them into
 /%{_var}/www/html/ throwing away the fact they were placed in css directory.
 
 Sending you patch which fixes the problem.

Ah yes, I see. Thank you very much for testing it! I applied your patch and 
hope that
the package builds and works fine apart from that.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Less compiler package to use

2013-10-22 Thread Johannes Renner
On 10/22/2013 02:07 PM, Tomáš Kašpárek wrote:
 I've locally merged contents of bootstrap-css branch and apart from 
 spacewalk-branding all other
 packages (spacewalk-java and spacewalk-web) all packages were build correctly 
 and all packages were
 correctly upgraded.
 
 I've noticed that some icons (e.g. in legend text on the left) are missing, 
 also search button is
 missing some picture or text. On pages like 
 /rhn/channels/ChannelDetail.do?cid=101 (Channels - pick
 one from displayed channels) old update button is used (not the green 
 bootstrap one). I've found
 some small issues but I think they'll be fixed as it's still work in progress.
 
 Apart from these issues bootstrap is working and I haven't found any major 
 issue like internal
 server errors.

Thanks, your review is giving us some hints about what else we might be missing
in the *.spec files of relevant packages. We will look into it.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] Less compiler package to use

2013-10-21 Thread Johannes Renner
Hello,

While working on the Spacewalk redesign, we so far used less.js client-side. 
This is fine for
development purposes, but it's however not the recommended way to go for 
production environments,
see [1]. Rather we probably want to compile the spacewalk.less into 
spacewalk.css during the rpm
build of spacewalk-branding. This means: 'BuildRequire' a less compiler, call 
it during build and
install only the compiled *.css onto the server.

Now, since I would want to modify the .spec of spacewalk-branding accordingly, 
my question is the
following: Do you have any preference of a specific compiler package to use? It 
would be good if
we use a package that either already exists, or does not have many 
dependencies, so it is easy to
build for all of the relevant distributions.

The most valuable options as far as I can see are:

- nodejs-less (there is nodejs-less in fedora, but it depends on nodejs, of 
course)
- lesscss-engine (Java wrapper around less.js, no dependency on nodejs)
- install less as ruby gem (requires ruby, I didn't test this one)
- python-lesscpy (re-implementation based on Python Lex-Yacc, fails for our 
current .less)

Let me know what you think, especially tell me if I am missing any other good 
options here.
I personally would go for lesscss-engine: It's based on less.js and it needs 
only rhino and
some jakarta commons libs. We build it for openSUSE already, see [2].

Thanks and regards,
Johannes

[1] http://lesscss.org/#usage
[2] https://build.opensuse.org/package/show/home:dmacvicar/lesscss-engine

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Less compiler package to use

2013-10-21 Thread Johannes Renner
On 10/21/2013 01:06 PM, Michael Mraka wrote:
 Johannes Renner wrote:
 % Hello,
 % 
 % While working on the Spacewalk redesign, we so far used less.js 
 client-side. This is fine for
 % development purposes, but it's however not the recommended way to go for 
 production environments,
 % see [1]. Rather we probably want to compile the spacewalk.less into 
 spacewalk.css during the rpm
 % build of spacewalk-branding. This means: 'BuildRequire' a less compiler, 
 call it during build and
 % install only the compiled *.css onto the server.
 % 
 % Now, since I would want to modify the .spec of spacewalk-branding 
 accordingly, my question is the
 % following: Do you have any preference of a specific compiler package to 
 use? It would be good if
 % we use a package that either already exists, or does not have many 
 dependencies, so it is easy to
 % build for all of the relevant distributions.
 % 
 % The most valuable options as far as I can see are:
 % 
 % - nodejs-less (there is nodejs-less in fedora, but it depends on nodejs, of 
 course)
 % - lesscss-engine (Java wrapper around less.js, no dependency on nodejs)
 % - install less as ruby gem (requires ruby, I didn't test this one)
 % - python-lesscpy (re-implementation based on Python Lex-Yacc, fails for our 
 current .less)
 
 Hello Johannes,
 
 nodejs-less and python-lesscpy are already available both in Fedora and RHEL
 (via EPEL) so we prefer one of these.

Fine, I modified the .spec file of spacewalk-branding to depend on nodejs-less. 
We just pushed the
branch to the upstream repo and it would be really nice if someone of you could 
review my commits
and maybe even try out if the package builds correctly?

I'm talking about these commits:

- cb3d206cf8c46f6afaccb03fb2060da16ab45ed8
- 12ce0476ba547fade1cbe3819e77fd560de7fdb0

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] ISE when changing user details on /rhn/users/UserDetails.do?uid=2

2013-10-17 Thread Johannes Renner
On 10/17/2013 04:17 PM, Tomáš Kašpárek wrote:
 Hello,
 
 I've merged contents of bootstrap-css branch with current Spacewalk master, 
 built it and applied.
 I've noticed that changing user details as admin produces ISE.
 
 Sending you patch which fixes the problem

Thank you very much for the hint and the patch!
Applied as 8ad61979b5c9b1b2a7e7d5fa954699b32909e7f3.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Make taskomatic max memory configurable via rhn.conf

2013-09-13 Thread Johannes Renner
Hello,

the attached patch is supposed to enable users to override the taskomatic config
option 'wrapper.java.maxmemory' via the global rhn.conf file. As the key I chose
'taskomatic.maxmemory', but feel free to propose something else in case you 
don't
like it.

Some of our users had issues with the default value of 1024 and increasing the
value fixes their problems. Further, since nobody is supposed to edit the 
default
config files in /usr/share/rhn/config-defaults/, there is a necessity for such a
patch. I hope you like the idea.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From bff7c320b8b6a20f1f829982a166077e07989afa Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 13 Sep 2013 11:11:13 +0200
Subject: [PATCH] Make taskomatic max memory configurable via rhn.conf

---
 java/scripts/taskomatic | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/java/scripts/taskomatic b/java/scripts/taskomatic
index 8eda04b..36bd9be 100755
--- a/java/scripts/taskomatic
+++ b/java/scripts/taskomatic
@@ -146,6 +146,12 @@ then
 fi
 fi
 
+# Read 'wrapper.java.maxmemory' from rhn.conf as 'taskomatic.maxmemory'
+MAX_MEMORY=$( grep -E -m1 ^taskomatic.maxmemory[[:space:]]*= /etc/rhn/rhn.conf | sed s/^taskomatic.maxmemory[[:space:]]*=[[:space:]]*\(.*\)/\1/ || echo  )
+if [ ! -z ${MAX_MEMORY} ]; then
+  MAX_MEMORY=wrapper.java.maxmemory=${MAX_MEMORY}
+fi
+
 getpid() {
 if [ -f $PIDFILE ]
 then
@@ -186,9 +192,9 @@ console() {
 then
 if [ X$IGNORE_SIGNALS = X ]
 then
-exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE
+exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE
 else
-exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE
+exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE
 fi
 else
 echo $APP_LONG_NAME is already running.
@@ -205,16 +211,16 @@ start() {
 then
 if [ X$RUN_AS_USER = X ]
 then
-exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE wrapper.daemonize=TRUE
+exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE wrapper.daemonize=TRUE
 else
-/sbin/runuser -m $RUN_AS_USER -c exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE wrapper.daemonize=TRUE
+/sbin/runuser -m $RUN_AS_USER -c exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE wrapper.daemonize=TRUE
 fi
 else
 if [ X$RUN_AS_USER = X ]
 then
-exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE wrapper.ignore_signals=TRUE wrapper.daemonize=TRUE
+exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE wrapper.ignore_signals=TRUE wrapper.daemonize=TRUE
 else
-/sbin/runuser -m $RUN_AS_USER -c exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE wrapper.ignore_signals=TRUE wrapper.daemonize=TRUE
+/sbin/runuser -m $RUN_AS_USER -c exec $CMDNICE $WRAPPER_CMD $WRAPPER_CONF $MAX_MEMORY wrapper.pidfile=$PIDFILE wrapper.anchorfile=$ANCHORFILE wrapper.ignore_signals=TRUE wrapper.daemonize=TRUE
 fi
 fi
 else
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Make CSV Separator Character Configurable

2013-08-20 Thread Johannes Renner
On 08/19/2013 07:46 PM, Stephen Herr wrote:
 Wikipedia tells me that I am wrong, and that people commonly use separators 
 other than commas in CSV
 files. So never mind what I said.

Yes, exactly. I read that up as well before I wrote the code. There simply is 
no standard for CSV
files. If it bothers you, you can always assume that it stands for 
Character-separated values,
but I think we should keep on calling it CSV and *.csv.

Regards,
Johannes

 On 08/19/2013 01:41 PM, Stephen Herr wrote:
 I don't necessarily have a problem with the idea, but I will comment that 
 CSVs are by definition
 Comma Separated Value files. Excel and every other spreadsheet in the 
 world should have an
 option to import from comma-separated lists. If we do want to change the 
 delimiter then we should
 call it something other than *.csv.

 -Stephen

 On 08/19/2013 08:21 AM, Johannes Renner wrote:
 Hey,

 here is a small feature patch that allows users to choose semicolon (;) 
 as an
 alternative to comma (,) for the delimiter in all downloadable CSV files 
 that
 we generate from the webapp. This seems to be helpful for importing those 
 files
 into MS Excel for example.

 Unfortunately we had to hack the schema source sanity check in order to not
 complain about a ; in the middle of a table definition.

 I hope you like the idea and the patch as well, let me know what you think.

 Thanks,
 Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Make CSV Separator Character Configurable

2013-08-19 Thread Johannes Renner
Hey,

here is a small feature patch that allows users to choose semicolon (;) as an
alternative to comma (,) for the delimiter in all downloadable CSV files that
we generate from the webapp. This seems to be helpful for importing those files
into MS Excel for example.

Unfortunately we had to hack the schema source sanity check in order to not
complain about a ; in the middle of a table definition.

I hope you like the idea and the patch as well, let me know what you think.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 7a446a37f1582321f432d097bd3dabdac8ecb9a2 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 19 Aug 2013 13:31:35 +0200
Subject: [PATCH 1/3] Allow users to change the CSV separator (schema changes)

---
 schema/spacewalk/common/tables/rhnUserInfo.sql  |  6 +-
 .../014-add-column-csv-separator.sql.oracle | 16 
 .../014-add-column-csv-separator.sql.postgresql | 17 +
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.oracle
 create mode 100644 schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.postgresql

diff --git a/schema/spacewalk/common/tables/rhnUserInfo.sql b/schema/spacewalk/common/tables/rhnUserInfo.sql
index 50989ad..005c840 100644
--- a/schema/spacewalk/common/tables/rhnUserInfo.sql
+++ b/schema/spacewalk/common/tables/rhnUserInfo.sql
@@ -63,7 +63,11 @@ CREATE TABLE rhnUserInfo
 DEFAULT (current_timestamp) NOT NULL,
 modifiedtimestamp with local time zone
 DEFAULT (current_timestamp) NOT NULL,
-preferred_localeVARCHAR2(8)
+preferred_localeVARCHAR2(8),
+csv_separator   CHAR(1)
+DEFAULT (',') NOT NULL
+CONSTRAINT rhn_user_info_csv_ck
+CHECK (csv_separator in (',',';'))
 )
 ENABLE ROW MOVEMENT
 ;
diff --git a/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.oracle b/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.oracle
new file mode 100644
index 000..2b1c796
--- /dev/null
+++ b/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.oracle
@@ -0,0 +1,16 @@
+--
+-- Copyright (c) 2013 SUSE
+--
+-- This software is licensed to you under the GNU General Public License,
+-- version 2 (GPLv2). There is NO WARRANTY for this software, express or
+-- implied, including the implied warranties of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+-- along with this software; if not, see
+-- http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+--
+
+-- Create new column for rhnUserInfo
+ALTER TABLE rhnUserInfo ADD csv_separator CHAR(1) DEFAULT (',') NOT NULL
+CONSTRAINT rhn_user_info_csv_ck
+CHECK (csv_separator in (',',';'));
+
diff --git a/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.postgresql b/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.postgresql
new file mode 100644
index 000..588b614
--- /dev/null
+++ b/schema/spacewalk/upgrade/spacewalk-schema-2.0-to-spacewalk-schema-2.1/014-add-column-csv-separator.sql.postgresql
@@ -0,0 +1,17 @@
+-- oracle equivalent source sha1 cfee4377c397d6a17de0fbbce988af873d88ec01
+--
+-- Copyright (c) 2013 SUSE
+--
+-- This software is licensed to you under the GNU General Public License,
+-- version 2 (GPLv2). There is NO WARRANTY for this software, express or
+-- implied, including the implied warranties of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+-- along with this software; if not, see
+-- http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+--
+
+-- Create new column for rhnUserInfo
+ALTER TABLE rhnUserInfo ADD csv_separator CHAR(1) DEFAULT (',') NOT NULL
+CONSTRAINT rhn_user_info_csv_ck
+CHECK (csv_separator in (',',';'));
+
-- 
1.8.1.4

From 49e2881bf54bb0273c85b9093587df3894a5576a Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 19 Aug 2013 14:02:18 +0200
Subject: [PATCH 2/3] Allow users to change the CSV separator

---
 java/code/src/com/redhat/rhn/common/util/CSVWriter.java | 17 ++---
 java/code/src/com/redhat/rhn/domain/user/User.java  | 12 
 .../src/com/redhat/rhn/domain/user/legacy/UserImpl.java | 10 ++
 .../com/redhat/rhn/domain/user/legacy/UserInfo.hbm.xml  |  1 +
 .../src/com/redhat/rhn/domain/user/legacy/UserInfo.java | 17

[Spacewalk-devel] [PATCH] Fix html not being escaped in package information

2013-08-05 Thread Johannes Renner
Hello,

here is another small bugfix patch fixing HTML not being properly escaped
in package description on this page:

https://hostname/rhn/software/packages/Details.do?pid=pid

Found that with a package where there was an email address in ...
notation that didn't show up.

Thanks and regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 63be1b2d99b4a594e3cd198565ea5a13197aae87 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 5 Aug 2013 15:42:53 +0200
Subject: [PATCH] Fix HTML not being escaped in package information

---
 .../redhat/rhn/frontend/action/rhnpackage/PackageDetailsAction.java  | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/PackageDetailsAction.java b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/PackageDetailsAction.java
index b94830c..cf9f7f8 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/PackageDetailsAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/PackageDetailsAction.java
@@ -30,6 +30,7 @@ import com.redhat.rhn.frontend.xmlrpc.NoSuchPackageException;
 import com.redhat.rhn.manager.download.DownloadManager;
 import com.redhat.rhn.manager.rhnpackage.PackageManager;
 
+import org.apache.commons.lang.StringEscapeUtils;
 import org.apache.struts.action.ActionForm;
 import org.apache.struts.action.ActionForward;
 import org.apache.struts.action.ActionMapping;
@@ -125,8 +126,8 @@ public class PackageDetailsAction extends RhnAction {
 request.setAttribute(pack, pkg);
 // description can be null.
 if (pkg.getDescription() != null) {
-request.setAttribute(description,
-pkg.getDescription().replace(\n, BR\n));
+String description = StringEscapeUtils.escapeHtml(pkg.getDescription());
+request.setAttribute(description, description.replace(\n, BR\n));
 }
 else {
 request.setAttribute(description,
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Fix ISE when UUID and host ID is null

2013-07-19 Thread Johannes Renner
Hello,

I found an error navigating to:

https://hostname/rhn/systems/VirtualSystemsList.do

In my setup there is only one registered system that does not have a UUID and 
also
the virtual host system is unknown to the Spacewalk server in this case:

- current.getUuid() == null
- current.getHostSystemId() == null

So I think we should not actually do 
current.setSystemId(current.getHostSystemId());
here. The attached patch fixes the problem for me (in [1]) and also makes sense 
IMO.

Thank you and regards,
Johannes

[1] VirtualSystemsListSetupAction.java

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 1e0f80e4b6f43ade890dd3dae2a71bb4544dff01 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 19 Jul 2013 13:39:51 +0200
Subject: [PATCH] Fix ISE when UUID and host ID is null

---
 .../rhn/frontend/action/systems/VirtualSystemsListSetupAction.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/systems/VirtualSystemsListSetupAction.java b/java/code/src/com/redhat/rhn/frontend/action/systems/VirtualSystemsListSetupAction.java
index b6252ad..5e8a0fe 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/systems/VirtualSystemsListSetupAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/systems/VirtualSystemsListSetupAction.java
@@ -63,7 +63,7 @@ public class VirtualSystemsListSetupAction extends BaseSystemListSetupAction {
 if (current.isFakeNode()) {
 continue;
 }
-else if (current.getUuid() == null) {
+else if (current.getUuid() == null  current.getHostSystemId() != null) {
 current.setSystemId(current.getHostSystemId());
 }
 else {
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Fix indentation on user preferences page

2013-07-12 Thread Johannes Renner
Hello,

the small patch in the attachment fixes the indentation on the user
preferences page (https://hostname/rhn/account/UserPreferences.do).

More specifically this should indent the checkboxes in the Overview
Start Page section, just to make it look more similar to the other
subsections.

Thanks and regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From b687d7444374ca60fdc6a8387e70013e3acb8f12 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 12 Jul 2013 11:33:30 +0200
Subject: [PATCH] Fix indentation on user preferences page

---
 java/code/webapp/WEB-INF/pages/common/fragments/user/preferences.jspf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/java/code/webapp/WEB-INF/pages/common/fragments/user/preferences.jspf b/java/code/webapp/WEB-INF/pages/common/fragments/user/preferences.jspf
index 0d5a099..a1bcddb 100644
--- a/java/code/webapp/WEB-INF/pages/common/fragments/user/preferences.jspf
+++ b/java/code/webapp/WEB-INF/pages/common/fragments/user/preferences.jspf
@@ -21,11 +21,13 @@
 h2bean:message key=preferences.jsp.startpagetitle//h2
 pbean:message key=preferences.jsp.startpagedesc//p
 
+div class=preference
 c:forEach items=${userPrefForm.map.possiblePanes} var=item
 html:multibox property=selectedPanes value=${item.value} disabled=${item.disabled} styleId=type_${item.value}/
 	label for=type_${item.value}${item.label}/label
 br /
 /c:forEach
+/div
 
 input type=hidden name=pxt:trap value=rhn:user_prefs_edit_cb /
 html:hidden property=uid /
-- 
1.8.1.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] SSH Push Feature Proposal

2013-04-24 Thread Johannes Renner
On 04/12/2013 01:24 PM, Jan Pazdziora wrote:
 Johannes,
 
 let me summarize some big picture impressions, without commenting on
 every detail.
 
 You propose to address two situations:
 
 1) clients in DMZ that cannot reach the server, with server able to
reach the clients;
 
 2) overloading Spacewalk when multiple actions get (auto)scheduled for
many clients and they get woken up by osa-dispatcher/jabberd/osad
all at the same time.
 
 Frankly, the first scenario does not sound that interesting to me.
 Access to and from DMZ is typically closed from/to all other networks
 as well and only opened in a very targeted fashion. The IT of that
 organization would still need to allow access _to_ the DMZ to sshd
 ports on those machines. You can always have Spacewalk Proxy in
 DMZ2, having client talk to the proxy and that proxy to the Spacewalk,
 if your IT does not want to open the ports in the DMZ configuration
 directly. In both cases, the HTTP requests run by rhn_check / yum will
 end up on that Spacewalk server and if there is a way to compromise
 that server that way, it will happen. I would still need to check the
 patches in details to see how you solve the problem of the client IP
 addresses as seen by the Spacewalk server being 127.0.0.1 for all
 those requests which is hardly something you'd like to see in
 production.
 
 You propose new scheduling service in taskomatic to initiate the SSH
 Push for client ... but we already have such a functionality, it's
 osa-dispatcher. So either we should get rid of osa-dispatcher and do
 even the jabber/osad based notifications from taskomatic, or we should
 stick with osa-dispatcher and not create very similar solution in
 Java.

I agree that duplicating such functionality is not a very good thing to
do and that it would make more sense to do jabber as well as ssh based
push notifications from within the same component of the software. On the
other hand I wonder why osa-dispatcher has been written in Python in the
first place? If there is something like taskomatic, shouldn't we put such
functionality in there rather than having yet another separate component?

 The second scenario is however much more important and interesting
 -- yes, the server will get overloaded if you use many osad-enabled
 clients, and we had Spacewalk users complaining about this on the
 mailing list in the past. However, is the cure really to allow ssh
 access from server to the clients? How about clients that are behind
 NAT, roaming, or in general unavailable?

I am not saying that SSH Push should be the cure for anything. Rather I
would consider it as just another contact method that can optionally be
used if it is suitable to anybody's needs. OSAD should IMO stay alive and
be available as a valid alternative. It should be the customer's choice.

The cure for the client complaints should rather be to make the existing
software more robust and stable in the first place, i.e. mitigating issues
with scalability. Taskomatic even offers some generic classes that can be
used to do threading and queuing (TaskQueue, QueueDriver and QueueWorker),
so isn't it actually a real option to port that osa-dispatcher
functionality over there?

 I would assume that the majority of the Spacewalk (and downstream
 products') installations has server accessible from clients because
 otherwise things would currently not work. If you completely reverse
 the style of operation, it will cause the disruption in our users'
 setups. And still, clients for which you won't be able or willing
 to enable the SSH Push functionality will not get the improvement
 in timely actions that don't put the server to its knees.

I don't think this would cause any disruptions. Actually it would even
enable users to administrate systems using Spacewalk where it was not
possible before. This means more users might even be attracted and also
existing users might use Spacewalk to maintain more systems than before.
You can even use ssh push to manage systems located in some public cloud
overseas, as long as you can login using ssh from the server.

 I would very much love to see improvements to the second problem which
 could be used by all *existing* clients of Spacewalk, even those that
 are behind NAT, independed from the SSH Push feature.
 
 Can the logic you propose to be put to taskomatic be put to
 osa-dispatcher, to throttle the number of clients which get invited
 to rhn_check?

Well, it's currently implemented in Java of course, and the first layer
of throttling is only queuing and threading, as mentioned before: There
is a (configurable) number of threads which specifies the number of
concurrent ssh connections allowed at a time. If there is actions to be
performed on more clients, the oldest ones in the schedule are treated
first, while the rest will be queued and revisited as soon as there is
free threads available. This was all done using existing classes from the
taskomatic framework.

The logic that was 

Re: [Spacewalk-devel] [PATCH] Kickstartable channels should contain the anaconda package

2013-04-05 Thread Johannes Renner
On 04/05/2013 08:52 AM, Jan Pazdziora wrote:
 Hmm, I wonder if the current logic is actually correct. It feel that
 a kickstartable channel (for the purpose of creating kickstart profile,
 for example) would be channel that has at least one kickstartable
 tree, or channel cloned from one that has the tree. Which is not
 a logic I can see in Channel.kickstartableChannels.
 
 If we changed the logic this way, that would work for SUSE as well,
 right? I'd much prefer to do it this way than hardcoding yet another
 name of package to the code base.
 
 Of course, creating new distribution should basically be allowed for
 any parent channel, it would seem.

Yes, this should work for SUSE as well. In this case we would need to
consider the install type of the tree (equals distribution?) though,
and make sure that it's != 'suse'. Agreed that such a solution would
be better than hardcoding the package name.

The original query should be suitable for creating new distributions.

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Kickstartable channels should contain the anaconda package

2013-04-04 Thread Johannes Renner
On 04/04/2013 04:31 PM, Jan Pazdziora wrote:
 On Thu, Apr 04, 2013 at 03:37:07PM +0200, Johannes Renner wrote:
 On 03/22/2013 02:55 PM, Michael Calmer wrote:
 Hi,

 please do not apply these patches. I found out, that it is not correct.
 It removes the channels at more places than needed.

 I guess the initial patch would be fine, as long as we add some additional
 small fixes on top. Here is another single patch containing everything.

 Basically what we are doing is to distinguish between kickstartable and
 autoinstallable channels. It would be really helpful though to get some
 feedback from you about using the anaconda package as a filter criteria.
 
 Can you elaborate on the purpose of this change in general? The current
 code uses the
 
   Channel.kickstartableChannels
 
 query everywhere (via getKickstartableChannels). The patch seems to
 change the semantics for all getKickstartableChannels(org)
 invocations. What is the purpose? Why should all the other invocations
 start to care about anaconda?

Sure, the problem is with the create a new kickstart profile wizard that
can be found if you navigate to Systems - Kickstart. The first step of
this wizard lets you decide on a base channel for the profile, where the
above query is used to determine the candidates. SUSE channels should *not*
be listed here, since it is not possible to have a kickstart profile with
a SUSE base channel. This leads to the conclusion that only those channels
should be listed, that actually support kickstarts (- kickstartable).

As far as we know, Anaconda is the package that should make a channel
kickstartable, since the whole kickstarting seems to be implemented in
there, right?

It's different though with the kickstart distributions. If you go there
and click create new distribution, you want to have those SUSE channels
listed a well. Distributions can be used with kickstart *as well as* with
autoyast profiles! That's why we would need to return the unfiltered list
here. We named this superset autoinstallable channels.

All other invocations should be expecting kickstartable channels only.

 Also, I'm not sure about the use of
 ChannelManager.listLatestPackagesEqual here -- we don't care about
 the latest at all, do we? The thing is happy about any package it
 finds so the best way is to just check for existence, without any
 newest computation needed.

I don't think there is actually logic implemented that would produce any
kind of overhead. It's all in the database as you can see if you look at
the used query (in Channel_queries.xml) latest_package_equal.

Also I saw in another place that looking up a package was done like this,
so this method appeared to be easier than anything else. It can be changed
of course, if you want. I agree that we don't need the latest version of a
package, but it looks like as soon as there is a record in the table
rhnChannelNewestPackage, we can assume this package is contained in the
channel.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Kickstartable channels should contain the anaconda package

2013-03-14 Thread Johannes Renner
On 03/13/2013 05:33 PM, Johannes Renner wrote:
 attached is a patch that should take care that only kickstartable channels 
 are actually
 returned by the getKickstartableChannels() method in the ChannelFactory class.

Argh, this fixes ConcurrentModificationException, sorry!

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From acaadc6ee2a54f2e803f33ff5d401559b2e35359 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 14 Mar 2013 17:52:15 +0100
Subject: [PATCH] Prevent from concurrent modification

---
 .../code/src/com/redhat/rhn/domain/channel/ChannelFactory.java |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java b/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
index 5509f2b..f60cfb4 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
@@ -35,6 +35,7 @@ import org.hibernate.criterion.CriteriaSpecification;
 import org.hibernate.criterion.Projections;
 import org.hibernate.criterion.Restrictions;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -631,6 +632,7 @@ public class ChannelFactory extends HibernateFactory {
  * @return List of Channel objects
  */
 public static ListChannel getKickstartableChannels(Org org) {
+ListChannel ret = new ArrayListChannel();
 Map params = new HashMap();
 params.put(org_id, org.getId());
 ListChannel channels = singleton.listObjectsByNamedQuery(
@@ -639,11 +641,11 @@ public class ChannelFactory extends HibernateFactory {
 for (Channel c : channels) {
 List packages = ChannelManager.listLatestPackagesEqual(c.getId(),
 ConfigDefaults.DEFAULT_ANACONDA_PACKAGE_NAME);
-if (packages.size() = 0) {
-channels.remove(c);
+if (packages.size()  0) {
+ret.add(c);
 }
 }
-return channels;
+return ret;
 }
 
 /**
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Kickstartable channels should contain the anaconda package

2013-03-13 Thread Johannes Renner
Hey,

attached is a patch that should take care that only kickstartable channels are 
actually
returned by the getKickstartableChannels() method in the ChannelFactory class.

In addition to the existing query, it is checked if the 'anaconda' package is 
included
in all of the returned channels. If that's not the right package to check for 
or not the
way to go in general, then we can fix it of course.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 0457b2ae9ed9dd83ef18df3dafa2885598038291 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 13 Mar 2013 17:00:01 +0100
Subject: [PATCH] Kickstartable channels should contain the anaconda
 package

---
 .../src/com/redhat/rhn/common/conf/ConfigDefaults.java|1 +
 .../src/com/redhat/rhn/domain/channel/ChannelFactory.java |   13 -
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/common/conf/ConfigDefaults.java b/java/code/src/com/redhat/rhn/common/conf/ConfigDefaults.java
index 651afe1..653c18e 100644
--- a/java/code/src/com/redhat/rhn/common/conf/ConfigDefaults.java
+++ b/java/code/src/com/redhat/rhn/common/conf/ConfigDefaults.java
@@ -114,6 +114,7 @@ public class ConfigDefaults {
 
 public static final String DEFAULT_KICKSTART_PACKAGE_NAME = spacewalk-koan;
 public static final String KICKSTART_PACKAGE_NAME = kickstart_package;
+public static final String DEFAULT_ANACONDA_PACKAGE_NAME = anaconda;
 
 public static final String MOUNT_POINT = mount_point;
 public static final String KICKSTART_MOUNT_POINT = kickstart_mount_point;
diff --git a/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java b/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
index 3c47018..5509f2b 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/ChannelFactory.java
@@ -14,6 +14,7 @@
  */
 package com.redhat.rhn.domain.channel;
 
+import com.redhat.rhn.common.conf.ConfigDefaults;
 import com.redhat.rhn.common.db.datasource.CallableMode;
 import com.redhat.rhn.common.db.datasource.DataResult;
 import com.redhat.rhn.common.db.datasource.ModeFactory;
@@ -25,6 +26,7 @@ import com.redhat.rhn.domain.kickstart.KickstartableTree;
 import com.redhat.rhn.domain.org.Org;
 import com.redhat.rhn.domain.rhnpackage.Package;
 import com.redhat.rhn.domain.user.User;
+import com.redhat.rhn.manager.channel.ChannelManager;
 
 import org.apache.log4j.Logger;
 import org.hibernate.Criteria;
@@ -631,8 +633,17 @@ public class ChannelFactory extends HibernateFactory {
 public static ListChannel getKickstartableChannels(Org org) {
 Map params = new HashMap();
 params.put(org_id, org.getId());
-return singleton.listObjectsByNamedQuery(
+ListChannel channels = singleton.listObjectsByNamedQuery(
 Channel.kickstartableChannels, params, false);
+// Only return channels containing the anaconda package
+for (Channel c : channels) {
+List packages = ChannelManager.listLatestPackagesEqual(c.getId(),
+ConfigDefaults.DEFAULT_ANACONDA_PACKAGE_NAME);
+if (packages.size() = 0) {
+channels.remove(c);
+}
+}
+return channels;
 }
 
 /**
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Support native kvm images in SUSE Studio integration

2013-02-06 Thread Johannes Renner
( extractedImagePath ):
diff --git a/client/tools/rhn-virtualization/scripts/studio-kvm-template.xml b/client/tools/rhn-virtualization/scripts/studio-kvm-template.xml
index 42ac7dd..73d7797 100644
--- a/client/tools/rhn-virtualization/scripts/studio-kvm-template.xml
+++ b/client/tools/rhn-virtualization/scripts/studio-kvm-template.xml
@@ -14,7 +14,7 @@
   devices
 emulator/usr/bin/qemu-kvm/emulator
 disk type='file' device='disk'
-  driver name='qemu' type='vmdk'/
+  driver name='qemu' type='%(imageType)s'/
   source file='%(disk)s'/
   target dev='hda' bus='ide'/
   address type='drive' controller='0' bus='0' unit='0'/
-- 
1.7.10.4

From df03b174f5b3f4c55d1868ee9f9c44686cdee66c Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 7 Jan 2013 16:58:34 +0100
Subject: [PATCH] Make images of type 'kvm' show up on the UI

---
 .../src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java b/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
index 45eb4e8..dea77e7 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/renderers/ImagesRenderer.java
@@ -49,7 +49,7 @@ public class ImagesRenderer extends BaseFragmentRenderer {
 public static final String ATTRIB_ERROR_MSG = errorMsg;
 
 // List of all valid image types
-private static ListString validImageTypes = Arrays.asList(vmx, xen);
+private static ListString validImageTypes = Arrays.asList(kvm, vmx, xen);
 
 // The URL of the page to render
 private static final String PAGE_URL =
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] List of installable packages: API vs. Web UI

2012-11-22 Thread Johannes Renner
On 11/08/2012 12:18 PM, Johannes Renner wrote:
 On 11/05/2012 12:03 PM, Tomas Lestach wrote:
 ... 
 If there's a bug in the API call, we of course fix it.
 But we try to keep the current API interface as is. Spacewalk users have 
 their 
 scripts using our API calls and we do not want to break the scripts by 
 renaming/deleting existing API calls. (Introducing of new sensefull APIs is 
 always good. :)

 So, it is important that the API behaves according to its name. It the name 
 and behavior do not match, we fix the behavior according to the name and can 
 introduce a new API in case the behavior of the original API was helpfull.

 However, internal naming has lower priority, because the users do not come 
 into contact with it, so it cannot be confusing for them. (But we of course 
 try to keep the naming sensefull as well.)
 
 Ok, I might come up with a patch these days, fixing the existing API call so
 it behaves like its name is suggesting + maybe add a new call to provide the
 previous behavior in addition to that.
 
 Thanks for the explanations,
 Johannes

Ok, here we go: The first patch should fix the query for the existing API call
so that actually only the very latest versions of packages will be returned,
as the name of the function suggests. (Note that the query might have been
working for you correctly before, but only in case all of packages and updates
are provided through one single channel, I guess).

Further, the query is internally renamed like this (in Package_queries.xml):

system_latest_all_available_packages - system_latest_available_packages

The second patch adds a new API call to list all versions and releases of
packages that can be installed to a given system:

system.list_all_installable_packages(key, sid)

Unfortunately the previous query could not be reused, since it didn't actually
return all of the versions and releases of a package (as noted above). It rather
returned only the max EVR version of each package in every channel the system
is subscribed to. This might have lead to correct behavior for list_latest, but
only in case all packages are provided through _one_ single channel. So the
second patch also contains a new query to handle the list_all_... case.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 84295bbb50bf32bfbec425d3aca516eeb8276d85 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 21 Nov 2012 18:21:20 +0100
Subject: [PATCH] Fix query for API call system.listLatestInstallablePackages

---
 .../common/db/datasource/xml/Package_queries.xml   |   33 +++-
 .../redhat/rhn/manager/system/SystemManager.java   |2 +-
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
index 44bc39d..38e7f4b 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
@@ -6,20 +6,19 @@
   /query
 /callable-mode
 
-mode name=system_latest_all_available_packages
+mode name=system_latest_available_packages
   query params=sid
 SELECT
 pn.name AS name,
-full_list.id AS id,
+pkg.id AS id,
 NVL((full_list.evr).version, ' ') AS version,
 NVL((full_list.evr).release, ' ') AS release,
 NVL((full_list.evr).epoch, ' ') AS epoch,
 NVL(full_list.arch_label, ' ') AS arch_label
   FROM  (
  SELECT  p.name_id name_id,
- p.id,
- p.evr_id,
  max(pe.evr) evr,
+ pa.id as arch_id,
  pa.label as arch_label
FROM  rhnPackageArch PA, rhnPackageEVR PE, rhnPackage P,
  rhnChannelNewestPackage CNP, rhnServerChannel SC
@@ -28,17 +27,23 @@ SELECT
 AND  cnp.package_id = p.id
 AND  p.evr_id = pe.id
 AND  p.package_arch_id = pa.id
-   GROUP BY  p.name_id, p.id, p.evr_id, pa.label
+   GROUP BY  p.name_id, pa.label, pa.id
) full_list,
-   rhnPackageName pn
- WHERE  full_list.name_id = pn.id
-   AND  NOT EXISTS (SELECT 1
-  FROM rhnServerPackage SP, rhnPackageEVR PE2
- WHERE SP.server_id = :sid
-   AND SP.name_id = full_list.name_id
-   AND SP.evr_id = PE2.id
-   AND PE2.evr gt;= full_list.evr)
-ORDER BY  UPPER(pn.name), full_list.evr
+   rhnPackage pkg
+   join rhnPackageName pn on pkg.name_id = pn.id
+   join rhnPackageEVR pevr on pkg.evr_id = pevr.id
+   join rhnChannelPackage CP on CP.package_id = pkg.id
+   join rhnServerChannel SC on SC.channel_id = CP.channel_id
+ WHERE full_list.name_id = pkg.name_id
+   AND full_list.evr = pevr.evr
+   AND full_list.arch_id = pkg.package_arch_id

[Spacewalk-devel] [PATCH] Errors with unrequired field Prefix

2012-11-15 Thread Johannes Renner
Hey again,

We found some errors with the prefix field of a user's properties. First, when 
creating
a new user with the UI, it is not possible to leave the prefix empty, it 
complains about
a non-valid value (even though an 'empty' value is actually contained in the 
prefixes
list). Further, when editing a user's details it is not possible to empty the 
prefix.
This is especially annoying if you used e.g. spacecmd to create a user without 
a prefix.
It is then later on not even possible to edit and save such a user without 
changing the
prefix. I reproduced these issues reliably on Oracle as well as on PG.

The attached patch fixes the problem by setting the apparently correct empty 
value of  ,
just in case it is empty (== ). The select in the form might even return it 
correctly
as  , but looks like it is maybe trimmed away somewhere.. This works for me 
now, but
I tested the fix on PG only.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 7e095b96cc5d104b76cb0121ed8dc89e62ce50eb Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 15 Nov 2012 15:21:55 +0100
Subject: [PATCH] Fix errors with unrequired field 'Prefix'

---
 .../rhn/frontend/action/user/UserEditActionHelper.java   |3 ++-
 .../src/com/redhat/rhn/manager/user/CreateUserCommand.java   |   10 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index e7726ad..3056834 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -67,7 +67,8 @@ public abstract class UserEditActionHelper extends RhnAction {
 targetUser.setFirstNames((String)form.get(firstNames));
 targetUser.setLastName((String)form.get(lastName));
 targetUser.setTitle((String)form.get(title));
-targetUser.setPrefix((String)form.get(prefix));
+String prefix = (String)form.get(prefix);
+targetUser.setPrefix(prefix.isEmpty() ?   : prefix);
 // Update PAM Authentication attribute
 updatePamAttribute(loggedInUser, targetUser, form);
 }
diff --git a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
index fa16d7c..9930aad 100644
--- a/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
+++ b/java/code/src/com/redhat/rhn/manager/user/CreateUserCommand.java
@@ -224,12 +224,16 @@ public class CreateUserCommand {
  * errors list.
  */
 private void validatePrefix() {
-if (user.getPrefix() != null) {
+String prefix = user.getPrefix();
+if (prefix != null) {
 // Make sure whether prefix is valid, if it is set
 SortedSet validPrefixes = LocalizationService.getInstance().availablePrefixes();
-if (!validPrefixes.contains(user.getPrefix())) {
+if (prefix.isEmpty()) {
+user.setPrefix( );
+}
+if (!validPrefixes.contains(prefix)) {
 errors.add(new ValidatorError(error.user_invalid_prefix,
-   user.getPrefix(), validPrefixes.toString()));
+   prefix, validPrefixes.toString()));
 }
 }
 }
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] WebUI HTTP proxy validator doesn't check for special characters

2012-11-14 Thread Johannes Renner
Hello,

I just extended the HostPortValidator class to check proxy hostnames against a
regex defining a valid character set. It should allow letters (all languages),
numbers, '.' and '-', but no whitespace or any special characters.

See my attached patch.

Greetings,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 9628af3c615588d24d6be31d999b19e0532fdf24 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 14 Nov 2012 14:54:35 +0100
Subject: [PATCH] Check hostnames for special characters and whitespace

---
 .../com/redhat/rhn/common/validator/HostPortValidator.java   |7 +++
 .../rhn/common/validator/test/HostPortValidatorTest.java |   10 ++
 2 files changed, 17 insertions(+)

diff --git a/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
index 0210d5b..2fc2322 100644
--- a/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
+++ b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
@@ -31,6 +31,9 @@ public class HostPortValidator {
 // Pattern to match IPv6 address in bracket notation
 private static final Pattern IPV6_BRACKETS = Pattern.compile(^\\[(.*)\\](:(\\d*))?$);
 
+// Allow letters (of all languages), numbers, '.' and '-'
+private static final Pattern HOSTNAME = Pattern.compile(^[\\p{L}\\p{N}.-]*$);
+
 // Private constructor
 private HostPortValidator() {
 }
@@ -85,6 +88,10 @@ public class HostPortValidator {
 // Validate IP addresses externally (v4 and v6)
 if (host.replaceAll([\\d\\.], ).isEmpty() || host.contains(:)) {
 isValidHost = isValidIP(host);
+} else {
+// Validate hostname charset
+Matcher matcher = HOSTNAME.matcher(host);
+isValidHost = matcher.matches() ? isValidHost : false;
 }
 boolean isValidPort = true;
 if (port != null) {
diff --git a/java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java b/java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java
index b934ec9..e4047a9 100644
--- a/java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java
+++ b/java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java
@@ -68,4 +68,14 @@ public class HostPortValidatorTest extends TestCase {
 assertFalse(HostPortValidator.getInstance().isValid(
 http://proxy.example.com:;));
 }
+
+public void testHostnameCharset() {
+assertTrue(HostPortValidator.getInstance().isValid(müller));
+assertTrue(HostPortValidator.getInstance().isValid(pröxy.com));
+
+assertFalse(HostPortValidator.getInstance().isValid(pröxy.com;));
+assertFalse(HostPortValidator.getInstance().isValid(pröxy com));
+assertFalse(HostPortValidator.getInstance().isValid(pro xy:));
+assertFalse(HostPortValidator.getInstance().isValid(p$r%ox!y=));
+}
 }
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] WebUI HTTP proxy validator doesn't check for special characters

2012-11-14 Thread Johannes Renner
On 11/14/2012 04:21 PM, Tomas Lestach wrote:
 On Wednesday 14 of November 2012 15:06:44 Johannes Renner wrote:
 Hello,

 I just extended the HostPortValidator class to check proxy hostnames against
 a regex defining a valid character set. It should allow letters (all
 languages), numbers, '.' and '-', but no whitespace or any special
 characters.

 See my attached patch.

 Greetings,
 Johannes
 
 Applied as: ebc61c00
 I just needed to commit 2 checkstyle fixes. :-)

Ah, sorry, I forgot to check (the style).
Thanks for pushing the commits!

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Quartz SimpleTrigger mis-fire situations

2012-11-12 Thread Johannes Renner
Hello,

When scheduling many repo syncs at a time (as 'mgr-ncc-sync' does it), we 
experienced
that not all of the channels were actually getting a refresh, try it with e.g. 
~ 30.
After some research we found that this is due to the 'repeat count' parameter 
given to
the SimpleTrigger constructor in TaskoQuartzHelper. This parameter seems to 
control the
so-called 'mis-fire' behavior in case there is e.g. no threads available at the 
actual
schedule time. There is usually not many threads available in this use case, 
since repo
sync tasks trigger the generation of new repository metadata as well.

Please see [1] or [2] for more details about handling of mis-fire situations in 
quartz.
The proposal is to use default values for repeat count and repeat interval, 
which will
lead to a more reasonable behavior (MISFIRE_INSTRUCTION_FIRE_NOW).

See the attached patch for a fix.

Regards,
Johannes

[1]
http://quartz-scheduler.org/api/1.8.5/org/quartz/SimpleTrigger.html#updateAfterMisfire%28org.quartz.Calendar%29
[2] http://java.dzone.com/articles/quartz-scheduler-misfire

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From e6172072966a2a7824770a48a033b064c787ac8b Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 9 Nov 2012 11:22:50 +0100
Subject: [PATCH] Fix quartz trigger initialization repeat count

---
 java/code/src/com/redhat/rhn/taskomatic/TaskoQuartzHelper.java |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/code/src/com/redhat/rhn/taskomatic/TaskoQuartzHelper.java b/java/code/src/com/redhat/rhn/taskomatic/TaskoQuartzHelper.java
index 191a5a9..864a7b8 100644
--- a/java/code/src/com/redhat/rhn/taskomatic/TaskoQuartzHelper.java
+++ b/java/code/src/com/redhat/rhn/taskomatic/TaskoQuartzHelper.java
@@ -67,7 +67,7 @@ public class TaskoQuartzHelper {
 Trigger trigger = null;
 if (isCronExpressionEmpty(schedule.getCronExpr())) {
 trigger = new SimpleTrigger(schedule.getJobLabel(),
-getGroupName(schedule.getOrgId()), 1, 1);
+getGroupName(schedule.getOrgId()));
 }
 else {
 try {
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Kickstart profiles error handling

2012-11-12 Thread Johannes Renner
On 11/12/2012 01:31 PM, Tomas Lestach wrote:
 Yes, my cobbler log contains some helpful info, but how to bring this
 excerpt to the UI if we are only reacting to a download that failed? I
 didn't dig deep into it, I just wanted to hide the html source of the 500
 error page. But I agree it would be much better to present such details to
 the user.
 
 I patched our cobbler, because in cause of a cheetah error it returned 
 different output for rhel5(mod_python) and rhel6(mod_wsgi). Now, we shall get 
 500 with the error in the response body in both cases.
 
 On WebUI for these cases - I removed the 'Download Kickstart File' link, 
 displayed an error message and changed the error backgrond to make sure the 
 user notices the problem.
 
 Let me know if you still would like to make some changes.
 

 Excerpt from cobbler.log:

 ...
 Here is the corresponding Cheetah code.
 ** I had to guess the line  column numbers, so they are probably incorrect:

 Line 47, column 8

 Line|Cheetah Code
 |-
 44  |
 45  |mkdir -p /tmp/rhn
 46  |
 47  |drives=$(list-harddrives | awk '{print $1}')
 ^
 48  |for disk in $drives; do
 49  |DISKS=$DISKS $(fdisk -l /dev/$disk | grep -v swap\|LVM\|Extended
 | awk '/^\/dev/{print $1}')
 50  |done
 ...

 BTW: This error occurred with a kickstart file that was generated using the
 wizard and pasted into the new profile form afterwards. The cheetah seems
 to complain about the $ signs in the bash code and wants me to escape them.
 Actually I would expect this to work, or not? Not so sure how the cheetah
 will distinguish between bash $ signs and kickstart variables?
 
 Right, this is the behavior I know since my first tries with Spacewalk 
 kickstart profiles. I mean it's because of the templating feature.
 You either have to escape the dollar signs or have to put the dollar 
 containing code into the #raw..#end block.
 

 Thanks,
 Johannes
 
 Related commits:
  747754ef
  74aa5047
  d1789a9e

Thank you, I merged your commits into our branch, but did not do any testing 
yet.
Two remarks regarding d1789a9e:

- I fixed a typo in one of the new messages: bellow - below
- I think we could omit the ';' after the '}' of the newly inserted css

A patch is attached.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 2062558127111d5fec6d1d2313d6d67e9b4208e0 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 12 Nov 2012 16:45:09 +0100
Subject: [PATCH] Fix typos

---
 branding/css/blue-docs.css   |2 +-
 .../src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/branding/css/blue-docs.css b/branding/css/blue-docs.css
index 1bec875..40909b2 100644
--- a/branding/css/blue-docs.css
+++ b/branding/css/blue-docs.css
@@ -139,7 +139,7 @@ pre,code,.guibutton,.keycap,.guilabel{font-size:0.9em;font-family:liberation mo
 .command .replaceable{color:#555;}
 pre{display:block;background-color:#f7f2d0;color:#333;overflow:auto;}
 pre code, code{white-space:normal;}
-pre.warning { padding: 1em; background-color: #d7d7d7; };
+pre.warning { padding: 1em; background-color: #d7d7d7; }
 
 /*Notifications*/
 div.note,div.tip ,div.important ,div.caution ,div.warning{
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
index cb8b0a8..2e558d6 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
@@ -8242,7 +8242,7 @@ Please note that some manual configuration of these scripts may still be require
 /context-group
   /trans-unit
   trans-unit id=kickstart.jsp.error.template_message
-sourceThere are errors in your kickstart template. Please check the error message bellow./source
+sourceThere are errors in your kickstart template. Please check the error message below./source
 context-group name=ctx
   context context-type=sourcefileKickstart Details/context
 /context-group
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Kickstart profiles error handling

2012-11-08 Thread Johannes Renner
On 11/07/2012 04:50 PM, Tomas Lestach wrote:
 On Tuesday 06 of November 2012 12:30:09 Johannes Renner wrote:
 Hello,

 this is a proposed extension to commit 1ea830a in master:

 Validation of a kickstart profile is currently done by downloading it from
 the locally running cobbler. In case of validation problems there is a
 DownloadException thrown that we can catch and work with.

 Therefore in case of an error, it makes IMO not very much sense to point to
 the 'Kickstart File' tab, where the downloaded file contents should
 actually be shown to the user. The cobbler returns a status 500 in this
 case, so what we see on this page is the source code of the apache error
 page. Further, the download link is of course not working.
 
 Right. I see your point.
 

 The attached patch contains the following proposed changes to the code:

 1. Change the error message text to point to the file contents on the
 'Details' tab 2. Hide the content on the 'Kickstart File' tab in case the
 download failed
 
 The problem with your patch is that you don't propagate any helpful (error) 
 message to the user. So if he uploads a kickstart file, that is considered 
 wrong for some reason, I - as an user - would like to know what was the 
 problem with the kickstart file.
 
 I know that the 500 page we display now isn't helpfull as well.
 
 I'll try check the code and see if I find something out.

Yes, my cobbler log contains some helpful info, but how to bring this excerpt to
the UI if we are only reacting to a download that failed? I didn't dig deep into
it, I just wanted to hide the html source of the 500 error page. But I agree it
would be much better to present such details to the user.

Excerpt from cobbler.log:

...
Here is the corresponding Cheetah code.
** I had to guess the line  column numbers, so they are probably incorrect:

Line 47, column 8

Line|Cheetah Code
|-
44  |
45  |mkdir -p /tmp/rhn
46  |
47  |drives=$(list-harddrives | awk '{print $1}')
^
48  |for disk in $drives; do
49  |DISKS=$DISKS $(fdisk -l /dev/$disk | grep -v swap\|LVM\|Extended | 
awk '/^\/dev/{print
$1}')
50  |done
...

BTW: This error occurred with a kickstart file that was generated using the 
wizard
and pasted into the new profile form afterwards. The cheetah seems to complain 
about
the $ signs in the bash code and wants me to escape them. Actually I would 
expect
this to work, or not? Not so sure how the cheetah will distinguish between bash 
$
signs and kickstart variables?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] List of installable packages: API vs. Web UI

2012-11-08 Thread Johannes Renner
On 11/05/2012 12:03 PM, Tomas Lestach wrote:
 ... 
 If there's a bug in the API call, we of course fix it.
 But we try to keep the current API interface as is. Spacewalk users have 
 their 
 scripts using our API calls and we do not want to break the scripts by 
 renaming/deleting existing API calls. (Introducing of new sensefull APIs is 
 always good. :)
 
 So, it is important that the API behaves according to its name. It the name 
 and behavior do not match, we fix the behavior according to the name and can 
 introduce a new API in case the behavior of the original API was helpfull.
 
 However, internal naming has lower priority, because the users do not come 
 into contact with it, so it cannot be confusing for them. (But we of course 
 try to keep the naming sensefull as well.)

Ok, I might come up with a patch these days, fixing the existing API call so
it behaves like its name is suggesting + maybe add a new call to provide the
previous behavior in addition to that.

Thanks for the explanations,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Kickstart profiles error handling

2012-11-06 Thread Johannes Renner
Hello,

this is a proposed extension to commit 1ea830a in master:

Validation of a kickstart profile is currently done by downloading it from the 
locally
running cobbler. In case of validation problems there is a DownloadException 
thrown that
we can catch and work with.

Therefore in case of an error, it makes IMO not very much sense to point to the
'Kickstart File' tab, where the downloaded file contents should actually be 
shown to the
user. The cobbler returns a status 500 in this case, so what we see on this 
page is the
source code of the apache error page. Further, the download link is of course 
not working.

The attached patch contains the following proposed changes to the code:

1. Change the error message text to point to the file contents on the 'Details' 
tab
2. Hide the content on the 'Kickstart File' tab in case the download failed

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 5e754d93de7475a88e38578faa0d32fce7581cdb Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 6 Nov 2012 12:10:34 +0100
Subject: [PATCH] Improve kickstart profiles error handling

---
 .../kickstart/KickstartFileDownloadAction.java |   12 +++--
 .../frontend/strings/jsp/StringResource_en_US.xml  |2 +-
 .../rhn/manager/kickstart/KickstartManager.java|4 +--
 .../kickstart/wizard/profile/advanced/download.jsp |   28 ++--
 4 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
index 7efc1c4..fd8acea 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
@@ -14,10 +14,13 @@
  */
 package com.redhat.rhn.frontend.action.kickstart;
 
+import com.redhat.rhn.common.localization.LocalizationService;
 import com.redhat.rhn.common.util.download.DownloadException;
 import com.redhat.rhn.common.validator.ValidatorError;
+import com.redhat.rhn.common.validator.ValidatorException;
 import com.redhat.rhn.domain.kickstart.KickstartData;
 import com.redhat.rhn.frontend.struts.RequestContext;
+import com.redhat.rhn.frontend.struts.RhnValidationHelper;
 import com.redhat.rhn.manager.kickstart.BaseKickstartCommand;
 import com.redhat.rhn.manager.kickstart.KickstartFileDownloadCommand;
 import com.redhat.rhn.manager.kickstart.KickstartManager;
@@ -81,8 +84,13 @@ public class KickstartFileDownloadAction extends BaseKickstartEditAction {
 KickstartManager.getInstance().renderKickstart(data)));
 }
 catch (DownloadException de) {
-request.setAttribute(FILEDATA,
-StringEscapeUtils.escapeHtml(de.getContent()));
+RhnValidationHelper.setFailedValidation(ctx.getRequest());
+try {
+ValidatorException.raiseException(kickstart.jsp.error.template_generation,
+LocalizationService.getInstance().getMessage(Details));
+} catch (ValidatorException ve) {
+getStrutsDelegate().saveMessages(request, ve.getResult());
+}
 }
 
 request.setAttribute(KSURL, KickstartUrlHelper.getCobblerProfileUrl(data));
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
index 0b21908..2010f1e 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
@@ -9050,7 +9050,7 @@ Please note that some manual configuration of these scripts may still be require
 
 
 trans-unit id=kickstart.jsp.error.template_generation
-sourceThere are errors in your kickstart template. Please check the '{0}' tab to determine the problem with the template./source
+sourceThere are errors in your kickstart template. Please check the file contents on the '{0}' tab to determine problems with the template./source
 context-group name=ctx
 context context-type=sourcefileKickstart Details/context
 /context-group
diff --git a/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java b/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
index ee0d13e1..055c658 100644
--- a/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
+++ b/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
@@ -122,13 +122,13 @@ public class KickstartManager extends BaseManager {
 ValidatorException.
 raiseException(kickstart.jsp.error.template_generation

Re: [Spacewalk-devel] List of installable packages: API vs. Web UI

2012-11-02 Thread Johannes Renner
On 11/01/2012 01:59 PM, Tomas Lestach wrote:
 On Wednesday 31 of October 2012 15:00:48 Johannes Renner wrote:
 Hello,

 We found that the list of installable packages for a given system differs if
 you either ask the API (via system.listLatestInstallablePackages) or if you
 go to the Web UI (Systems - [choose a system] - Software - Packages -
 Install).

 After looking into it, I found there is two different queries used for this
 (both to be found in Package_queries.xml):

 1. system_latest_all_available_packages (API)
 2. system_available_packages (Web UI)

 My questions:

 - Query (1.) seems to return all available versions of a package, which
 means 'all' rather than 'latest'? I think we could have two separate API
 calls, one for 'latest' and one for 'all', but 'latest_all'?
 
 Right, the naming is strange.
 
 - Shouldn't we use the same query for API and Web UI in order to get the
 same results here?
 
 I'd say we do not want to get the same results.
 As people are used to the WebUI, they may select and install any version of 
 an 
 available package. I wouldn't change that.
 
 If an API is called listLatestInstallablePackages, it shall definitelly 
 return 
 list of *latest* installable packages.
 
 But what we could do is we can introduce a 
 listAllInstallablePackages/listAvailableInstallablePackages API, that could 
 reuse the WebUI query. :-)

Haha, yes, good idea ;-)

So, IIUC this is the situation summarized:

The current API call needs to continue to exist as it is, but you agree that
the naming of the API call, as well as the query name, is strange.

To fix this, we would actually rename the existing API call to something more
'correct', like e.g. 'listAllInstallablePackages'. Further we could change the
semantics of 'listLatestInstallablePackages' in a way that it reuses the WebUI
query and thus will return the same results as the WebUI itself.

The only problem is that we are most likely not supposed to change the semantics
of an existing API call, right? Is this true, even if we considered such
'unexpected' behavior as a bug?

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Use braces for accessing composite types in PG

2012-11-02 Thread Johannes Renner
On 10/31/2012 05:00 PM, Tomas Lestach wrote:
 On Wednesday 31 of October 2012 14:38:49 Johannes Renner wrote:
 Hey,

 Ok, here is the patch to make the rewritten query work with PG as well (API
 call 'system.listLatestUpgradablePackages'). It's actually a follow up to
 this commit:

 4823565b2e634bc73010d366ff82c432630bca85

 
 Committed as: 594781171402e28f85cdcb9fe7e838e35288a505
 
 It was strange though when testing it: for most of my systems it returned
 really quick (with 0 results though), but I also had one system, where the
 query did not return at all, even after 1 hour! You might want to do some
 additional performance testing here. There was nothing special about this
 specific host though, it was even subscribed to a base channel only..
 
 Can you still reproduce it with that system?

I could reproduce it until I cherry-picked your most recent fix from master:

1168f61a9d16b42b533a21513c8538529862e745

The fixed query now runs reasonably fast and delivers results immediately for
that specific system. So I'd consider this issue as fixed.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Use braces for accessing composite types in PG

2012-10-31 Thread Johannes Renner
Hey,

Ok, here is the patch to make the rewritten query work with PG as well (API call
'system.listLatestUpgradablePackages'). It's actually a follow up to this 
commit:

4823565b2e634bc73010d366ff82c432630bca85

It was strange though when testing it: for most of my systems it returned really
quick (with 0 results though), but I also had one system, where the query did 
not
return at all, even after 1 hour! You might want to do some additional 
performance
testing here. There was nothing special about this specific host though, it was
even subscribed to a base channel only..

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From f48143d211baeb4161814ae5deb537aa57642c70 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 31 Oct 2012 12:18:02 +0100
Subject: [PATCH] Use braces for accessing composite types in PG

---
 .../rhn/common/db/datasource/xml/Package_queries.xml   |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
index 1198cef..29ab2cb 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Package_queries.xml
@@ -806,14 +806,14 @@ ORDER BY  UPPER(PN.name)
 mode name=system_upgradable_package_list_no_errata_info
   query params=sid
 SELECT  n.name,
-NVL(sp.evr.epoch, ' ') as from_epoch,
-NVL(sp.evr.version, ' ') as from_version,
-NVL(sp.evr.release, ' ') as from_release,
+NVL((sp.evr).epoch, ' ') as from_epoch,
+NVL((sp.evr).version, ' ') as from_version,
+NVL((sp.evr).release, ' ') as from_release,
 spa.label as from_arch,
 spa.label as arch,
-NVL(up.evr.epoch, ' ') as to_epoch,
-NVL(up.evr.version, ' ') as to_version,
-NVL(up.evr.release, ' ') as to_release,
+NVL((up.evr).epoch, ' ') as to_epoch,
+NVL((up.evr).version, ' ') as to_version,
+NVL((up.evr).release, ' ') as to_release,
 up.arch_label as to_arch,
 up.id as to_package_id
   FROM
-- 
1.7.10.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] List of installable packages: API vs. Web UI

2012-10-31 Thread Johannes Renner
Hello,

We found that the list of installable packages for a given system differs if 
you either ask
the API (via system.listLatestInstallablePackages) or if you go to the Web UI 
(Systems -
[choose a system] - Software - Packages - Install).

After looking into it, I found there is two different queries used for this 
(both to be found
in Package_queries.xml):

1. system_latest_all_available_packages (API)
2. system_available_packages (Web UI)

My questions:

- Query (1.) seems to return all available versions of a package, which means 
'all' rather
  than 'latest'? I think we could have two separate API calls, one for 'latest' 
and one for
  'all', but 'latest_all'?
- Shouldn't we use the same query for API and Web UI in order to get the same 
results here?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Schedule updates for the software update stack first

2012-10-19 Thread Johannes Renner
Hello,

attached is patches that will take care that, in case there is an update to the 
software update
stack, it will be scheduled before all other (regular) patch updates. This way 
we would like to
ensure, that if there is bugs in the update stack, they will be fixed before 
any other patch
updates will be installed. Relevant patches are found by inspecting the errata 
keywords, in this
case looking for restart_suggested.

Additionally the patches contain the following changes:

- All updates to the SW management stack will be combined into a single action 
for the client,
  which will also result in one call to zypper/yum on the client.
- Duplicated code is removed: The new algorithm exists only in one single 
place, which is
  ErrataManager.applyErrata().
- Make the details view show information about all errata attached to an action 
(2nd patch).

Sincerely,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From b27fdfcda077af488a17d6a2b7a18e133b01909a Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 18 Oct 2012 17:26:01 +0200
Subject: [PATCH] Schedule updates for software update stack first

---
 .../action/systems/ErrataConfirmSetupAction.java   |   26 ---
 .../frontend/strings/java/StringResource_en_US.xml |8 +++
 .../redhat/rhn/manager/action/ActionManager.java   |   54 +++---
 .../redhat/rhn/manager/errata/ErrataManager.java   |   75 
 4 files changed, 91 insertions(+), 72 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/systems/ErrataConfirmSetupAction.java b/java/code/src/com/redhat/rhn/frontend/action/systems/ErrataConfirmSetupAction.java
index 675ce52..3814ea7 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/systems/ErrataConfirmSetupAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/systems/ErrataConfirmSetupAction.java
@@ -15,9 +15,6 @@
 package com.redhat.rhn.frontend.action.systems;
 
 import com.redhat.rhn.common.util.DatePicker;
-import com.redhat.rhn.domain.action.Action;
-import com.redhat.rhn.domain.errata.Errata;
-import com.redhat.rhn.domain.errata.ErrataFactory;
 import com.redhat.rhn.domain.rhnset.RhnSet;
 import com.redhat.rhn.domain.server.Server;
 import com.redhat.rhn.domain.user.User;
@@ -27,7 +24,7 @@ import com.redhat.rhn.frontend.struts.RhnHelper;
 import com.redhat.rhn.frontend.struts.StrutsDelegate;
 import com.redhat.rhn.frontend.taglibs.list.helper.ListRhnSetHelper;
 import com.redhat.rhn.frontend.taglibs.list.helper.Listable;
-import com.redhat.rhn.manager.action.ActionManager;
+import com.redhat.rhn.manager.errata.ErrataManager;
 import com.redhat.rhn.manager.system.SystemManager;
 
 import org.apache.struts.action.ActionForm;
@@ -37,9 +34,13 @@ import org.apache.struts.action.ActionMessage;
 import org.apache.struts.action.ActionMessages;
 import org.apache.struts.action.DynaActionForm;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -113,16 +114,14 @@ public class ErrataConfirmSetupAction extends RhnAction implements Listable {
 Server server = SystemManager.lookupByIdAndUser(sid, user);
 RhnSet set = ErrataSetupAction.getSetDecl(sid).get(user);
 
-ListErrata errataList = ErrataFactory.listErrata(set.getElementValues());
-
+// Get the errata IDs
+SetLong errataList = set.getElementValues();
 if (server != null  !errataList.isEmpty()) {
- for (Errata e : errataList) {
- Action update = ActionManager.createErrataAction(user, e);
- ActionManager.addServerToAction(server.getId(), update);
- update.setEarliestAction(getStrutsDelegate().readDatePicker(form, date,
- DatePicker.YEAR_RANGE_POSITIVE));
- ActionManager.storeAction(update);
- }
+Date earliest = getStrutsDelegate().readDatePicker(form, date,
+DatePicker.YEAR_RANGE_POSITIVE);
+ListLong serverIds = Arrays.asList(server.getId());
+ListLong errataIds = new ArrayListLong(errataList);
+ErrataManager.applyErrata(user, errataIds, earliest, serverIds);
 
  ActionMessages msg = new ActionMessages();
  Object[] args = new Object[3];
@@ -153,7 +152,6 @@ public class ErrataConfirmSetupAction extends RhnAction implements Listable {
 mapping.findForward(RhnHelper.DEFAULT_FORWARD), params);
 }
 
-
 /**
  * Makes a parameter map containing request params that need to
  * be forwarded on to the success mapping.
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/java

Re: [Spacewalk-devel] [PATCH] Validate proxy format on general config page

2012-09-18 Thread Johannes Renner
On 09/14/2012 02:32 PM, Tomas Lestach wrote:
[...]
 
 Hi Johannes,
 
 the patch look good.
 
 However looking at [1] I see the code is licensed under the Apache License, 
 Version 2.0. I'm afraid this version isn't compatible with the Spacewalk's 
 GPLv2.
 Or is it? Cliff?

I reworked the code again and also simplified the regex, so I guess it could
be fine now? Attached is another version of the patch.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From ebd7f06c7332a44974b95f51146e59a527356f29 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 18 Sep 2012 14:17:11 +0200
Subject: [PATCH] Validate proxy format on general config page

---
 .../rhn/common/validator/HostPortValidator.java|  134 
 .../validator/test/HostPortValidatorTest.java  |   63 +
 .../action/satellite/GeneralConfigAction.java  |   12 +-
 .../frontend/strings/java/StringResource_en_US.xml |8 ++
 4 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
 create mode 100644 java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java

diff --git a/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
new file mode 100644
index 000..cff8ad7
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
@@ -0,0 +1,134 @@
+/**
+ * Copyright (c) 2012 Novell
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.common.validator;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Input validation for host[:port], where the host part can be either an IP address
+ * (IPv4 or IPv6) or a hostname.
+ */
+public class HostPortValidator {
+
+// Singleton instance
+private static HostPortValidator instance;
+
+// Pattern to match IPv6 address in bracket notation
+private static final Pattern IPV6_BRACKETS = Pattern.compile(^\\[(.*)\\](:(\\d*))?$);
+
+// Private constructor
+private HostPortValidator() {
+}
+
+/**
+ * Return the singleton instance of {@link HostPortValidator}.
+ * @return {@link HostPortValidator} instance
+ */
+public static HostPortValidator getInstance() {
+if (instance == null) {
+instance = new HostPortValidator();
+}
+return instance;
+}
+
+/**
+ * Return true if the given string is a valid host[:port] representation.
+ * @param hostPort
+ * @return true if hostPort represents a valid host and port, else false.
+ */
+public boolean isValid(String hostPort) {
+if (hostPort == null || hostPort.isEmpty()) {
+return false;
+}
+String host;
+String port;
+
+if (hostPort.startsWith([)) {
+  // Parse an IPv6 address in bracket notation
+  Matcher matcher = IPV6_BRACKETS.matcher(hostPort);
+  if (!matcher.matches()) {
+  return false;
+  }
+  host = matcher.group(1);
+  port = matcher.group(3);
+}
+else {
+  int colonIndex = hostPort.indexOf(':');
+  if (colonIndex != -1  colonIndex == hostPort.lastIndexOf(':')) {
+// Split into host:port
+host = hostPort.substring(0, colonIndex);
+port = hostPort.substring(colonIndex + 1);
+  }
+  else {
+host = hostPort;
+port = null;
+  }
+}
+
+// Validate host and port separately
+boolean isValidHost = true;
+// Validate IP addresses externally (v4 and v6)
+if (host.replaceAll([\\d\\.], ).isEmpty() || host.contains(:)) {
+isValidHost = isValidIP(host);
+}
+boolean isValidPort = true;
+if (port != null) {
+isValidPort = isValidPort(port);
+}
+
+return isValidHost  isValidPort;
+}
+
+/**
+ * Validate IP address format for a given string.
+ * @param ipString
+ * @return true if the given string is a valid IP, else false.
+ */
+private boolean isValidIP(String ipString) {
+boolean ret = false;
+if (ipString != null

Re: [Spacewalk-devel] [PATCH] Validate proxy format on general config page

2012-09-14 Thread Johannes Renner
On 09/12/2012 05:51 PM, Tomas Lestach wrote:
[...]
 Anyway, please, suppose Spacewalk is IPv6 ready, so if you allow IPv4 
 addresses, please allow IPv6 addresses as well.
 In case it turns out something doesn't work as expected, we'd face the issue.
 
 I'll be happy to commit your patch, when it allows also IPv6 addresses.

Ok, so here is a new implementation of the validator supporting IPv6 addresses
as well. Please take a look at the unit tests to see how it will behave.

The algorithm is similar to the one in [1] (see the fromString() method there)
and the BRACKET_PATTERN is taken from there as well. Do you think we need to
put a reference or something?

Otherwise if you are satisfied with this one we might want to use it on other
pages as well, like e.g. the client HTTP proxy here:

- https://hostname/rhn/admin/config/BootstrapConfig.do

Regards,
Johannes

[1]
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/net/HostAndPort.java

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 8f2afb12a36939da36856dff630d09746b9cd8ff Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 14 Sep 2012 11:26:34 +0200
Subject: [PATCH] Validate proxy format (host:port) on general config page

---
 .../rhn/common/validator/HostPortValidator.java|  134 
 .../validator/test/HostPortValidatorTest.java  |   57 +
 .../action/satellite/GeneralConfigAction.java  |   12 ++-
 .../frontend/strings/java/StringResource_en_US.xml |8 ++
 4 files changed, 210 insertions(+), 1 deletions(-)
 create mode 100644 java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
 create mode 100644 java/code/src/com/redhat/rhn/common/validator/test/HostPortValidatorTest.java

diff --git a/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
new file mode 100644
index 000..e2dee88
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
@@ -0,0 +1,134 @@
+/**
+ * Copyright (c) 2012 Novell
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.common.validator;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Input validation for host[:port], where the host part can be either an IP address
+ * (IPv4 or IPv6) or a hostname.
+ */
+public class HostPortValidator {
+
+// Singleton instance
+private static HostPortValidator instance;
+
+// Pattern to match IPv6 address in bracket notation
+private static final Pattern BRACKET_PATTERN = Pattern.compile(^\\[(.*:.*)\\](?::(\\d*))?$);
+
+// Private constructor
+private HostPortValidator() {
+}
+
+/**
+ * Return the singleton instance of {@link HostPortValidator}.
+ * @return {@link HostPortValidator} instance
+ */
+public static HostPortValidator getInstance() {
+if (instance == null) {
+instance = new HostPortValidator();
+}
+return instance;
+}
+
+/**
+ * Return true if the given string is a valid host[:port] representation.
+ * @param hostPort
+ * @return true if hostPort represents a valid host and port, else false.
+ */
+public boolean isValid(String hostPort) {
+if (hostPort == null || hostPort.isEmpty()) {
+return false;
+}
+String host;
+String port = null;
+
+if (hostPort.startsWith([)) {
+  // Try to parse IPv6 address in bracket notation
+  Matcher matcher = BRACKET_PATTERN.matcher(hostPort);
+  if (!matcher.matches()) {
+  return false;
+  }
+  host = matcher.group(1);
+  port = matcher.group(2);
+}
+else {
+  int colonPos = hostPort.indexOf(':');
+  if (colonPos = 0  hostPort.indexOf(':', colonPos + 1) == -1) {
+// Split into host:port
+host = hostPort.substring(0, colonPos);
+port = hostPort.substring(colonPos + 1);
+// There is a colon at the end, but no port
+if (port.isEmpty()) {
+return false;
+}
+  }
+  else {
+// No port is given
+host = hostPort

[Spacewalk-devel] [PATCH] Validate proxy format on general config page

2012-09-10 Thread Johannes Renner
Hey,

Here is a patch for adding some validation to the HTTP proxy field on the 
general
config page (https://hostname/rhn/admin/config/GeneralConfig.do).

This validator will allow FQDN or FQDN:port only, while a simple hostname will 
not
pass. IPv4 addresses will pass, but IPv6 won't. Feel free to propose changes, 
this
can be done in many different ways. I just went for an easy approach that reuses
some existing and tested code, but we can also use a regex if you prefer that.

BTW: Does anybody know why all error messages on that particular page keep on
appearing twice? Or it's not the case for you? I might look after this bug as 
well..

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 8ecd2908bb32bbce565503176b6fe937e851a2de Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 10 Sep 2012 11:31:13 +0200
Subject: [PATCH] Validate proxy format on general config page

---
 .../rhn/common/validator/HostPortValidator.java|   37 
 .../action/satellite/GeneralConfigAction.java  |   14 ++--
 .../frontend/strings/java/StringResource_en_US.xml |8 
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java

diff --git a/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
new file mode 100644
index 000..a52449a
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/common/validator/HostPortValidator.java
@@ -0,0 +1,37 @@
+/**
+ * Copyright (c) 2012 Novell
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+package com.redhat.rhn.common.validator;
+
+import org.apache.commons.validator.UrlValidator;
+
+/**
+ * Simple host[:port] validation reusing {@link UrlValidator} internals.
+ */
+public class HostPortValidator extends UrlValidator {
+
+// Singleton instance
+private static HostPortValidator instance;
+
+public static HostPortValidator getInstance() {
+if (instance == null) {
+instance = new HostPortValidator();
+}
+return instance;
+}
+
+public boolean isValidHostPort(String hostPort) {
+return isValidAuthority(hostPort);
+}
+}
diff --git a/java/code/src/com/redhat/rhn/frontend/action/satellite/GeneralConfigAction.java b/java/code/src/com/redhat/rhn/frontend/action/satellite/GeneralConfigAction.java
index 53cb5f2..2287b64 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/satellite/GeneralConfigAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/satellite/GeneralConfigAction.java
@@ -16,6 +16,7 @@ package com.redhat.rhn.frontend.action.satellite;
 
 import com.redhat.rhn.common.conf.Config;
 import com.redhat.rhn.common.conf.ConfigDefaults;
+import com.redhat.rhn.common.validator.HostPortValidator;
 import com.redhat.rhn.common.validator.ValidatorError;
 import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.struts.RequestContext;
@@ -213,7 +214,16 @@ public class GeneralConfigAction extends BaseConfigAction {
  */
 private ActionErrors validateForm(DynaActionForm form) {
 ActionErrors errors = new ActionErrors();
-String email = (String) form.get(translateFormPropertyName(traceback_mail));
+
+// Check if proxy is given as host:port
+String proxy = (String) form.get(
+translateFormPropertyName(server.satellite.http_proxy));
+HostPortValidator validator = HostPortValidator.getInstance();
+if (!(proxy.equals() || validator.isValidHostPort(proxy))) {
+errors.add(ActionMessages.GLOBAL_MESSAGE,
+new ActionMessage(error.proxy_invalid));
+}
+
 String password = (String) form.get(
translateFormPropertyName(server.satellite.http_proxy_password));
 String confirmationPassword = (String) form.get(
@@ -234,6 +244,4 @@ public class GeneralConfigAction extends BaseConfigAction {
 
 return errors;
 }
-
 }
-
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_en_US.xml
index 86e3bd7..507e32d 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_en_US.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings

[Spacewalk-devel] [PATCH] CVEs missing in the security patches listing (on Oracle 11)

2012-08-17 Thread Johannes Renner
Hey,

apparently the method getColumnLabel() of class ResultSetMetaData returns the 
column
label in uppercase with Oracle 11 (using ojdbc5.jar). Therefore an 
equals(cve) in
SecurityErrataOverview.java is not sufficient anymore and leads to an empty 
CVEs
column in the listing of relevant security errata:

https://hostname/rhn/errata/RelevantSecurityErrata.do

The attached patch fixes the problem by replacing equals with 
equalsIgnoreCase().

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From dd9bf777412f470c7a8fee58408b495036f0d55b Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 17 Aug 2012 12:06:49 +0200
Subject: [PATCH] Fix missing CVEs in patches listing with Oracle 11

---
 .../rhn/frontend/dto/SecurityErrataOverview.java   |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/dto/SecurityErrataOverview.java b/java/code/src/com/redhat/rhn/frontend/dto/SecurityErrataOverview.java
index 070d561..62a0db1 100644
--- a/java/code/src/com/redhat/rhn/frontend/dto/SecurityErrataOverview.java
+++ b/java/code/src/com/redhat/rhn/frontend/dto/SecurityErrataOverview.java
@@ -52,7 +52,7 @@ public class SecurityErrataOverview extends ErrataOverview
 // expected errata_cves_elab returns 2 columns
 if (columnCount  3) {
 for (int i = 1; i = columnCount; i++) {
-if (meta.getColumnLabel(i).equals(cve)) {
+if (meta.getColumnLabel(i).equalsIgnoreCase(cve)) {
 String cve = rs.getString(cve);
 if (cve != null) {
 addCve(cve);
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] CVEs missing in the security patches listing (on Oracle 11)

2012-08-17 Thread Johannes Renner
On 08/17/2012 01:20 PM, Jan Pazdziora wrote:
 On Fri, Aug 17, 2012 at 01:02:38PM +0200, Johannes Renner wrote:
 Hey,

 apparently the method getColumnLabel() of class ResultSetMetaData returns 
 the column
 label in uppercase with Oracle 11 (using ojdbc5.jar). Therefore an 
 equals(cve) in
 SecurityErrataOverview.java is not sufficient anymore and leads to an empty 
 CVEs
 column in the listing of relevant security errata:
 
 Is there a configuration (session level or something) option which
 would make it lowercase? Because I assume that if this is the case, we
 will have many more places where we simply expect the column name to
 be lowercase, so I'd rather make getColumnLabel keep its old
 behaviour.

Not that I would be aware of such an option.

Further note that this is the only call to getColumnLabel/getColumnName, at 
least in
the Java parts of spacewalk. Not sure about similar stuff in Python, Perl and 
others,
but I don't think this would cause so many additional problems.

Another possible fix that I just tried does unfortunately not fix the problem: 
I saw
that in the elaborator query CVE is written in uppercase, so I lowercased it 
there,
but it doesn't work (see errata_cves_elab in Errata_queries.xml). The Oracle 
seems
to return it in uppercase anyways, no matter how it is written in the query ...

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Insert pxt session errors on postgres

2012-08-15 Thread Johannes Renner
Hello,

We found a problem with an sql statement not working on postgresql, causing
errors when trying to browse perl pages while not logged in to spacewalk.

To reproduce, try to navigate to one of these pages when not logged in on a
postgres based installation:

https://hostname/help/copyright.pxt
https://hostname/help/about.pxt

Attached please find a patch for the issue.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 76dfe76b7cb8a7882951849ed537a66154aa86cc Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 15 Aug 2012 15:23:59 +0200
Subject: [PATCH] Fix insert pxt session for postgres when not logged in

---
 .../postgres/procs/create_pxt_session.sql  |2 +-
 .../124-create_pxt_session.sql.oracle  |1 +
 .../124-create_pxt_session.sql.postgresql  |   35 
 3 files changed, 37 insertions(+), 1 deletions(-)
 create mode 100644 schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.oracle
 create mode 100644 schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.postgresql

diff --git a/schema/spacewalk/postgres/procs/create_pxt_session.sql b/schema/spacewalk/postgres/procs/create_pxt_session.sql
index 3b3417f..aafeb5f 100644
--- a/schema/spacewalk/postgres/procs/create_pxt_session.sql
+++ b/schema/spacewalk/postgres/procs/create_pxt_session.sql
@@ -29,7 +29,7 @@ begin
 perform pg_dblink_exec(
 'insert into PXTSessions (id, value, expires, web_user_id) values (' ||
 l_id || ', ' || coalesce(quote_literal(p_value), 'NULL') ||
-', ' || p_expires || ', ' || p_web_user_id || '); commit');
+', ' || p_expires || ', ' || coalesce(quote_literal(p_web_user_id), 'NULL') || '); commit');
 
 	return l_id;
 end;
diff --git a/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.oracle b/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.oracle
new file mode 100644
index 000..b7b00b1
--- /dev/null
+++ b/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.oracle
@@ -0,0 +1 @@
+-- This file is intentionally left empty.
diff --git a/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.postgresql b/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.postgresql
new file mode 100644
index 000..567be73
--- /dev/null
+++ b/schema/spacewalk/upgrade/spacewalk-schema-1.7-to-spacewalk-schema-1.8/124-create_pxt_session.sql.postgresql
@@ -0,0 +1,35 @@
+-- oracle equivalent source sha1 b14267384bc104605623a41b755e68e0103b5aa8
+--
+-- Copyright (c) 2008--2012 Red Hat, Inc.
+--
+-- This software is licensed to you under the GNU General Public License,
+-- version 2 (GPLv2). There is NO WARRANTY for this software, express or
+-- implied, including the implied warranties of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+-- along with this software; if not, see
+-- http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+--
+-- Red Hat trademarks are not licensed under GPLv2. No permission is
+-- granted to use or replicate Red Hat trademarks that are incorporated
+-- in this software or its documentation.
+--
+--
+--
+--
+
+create or replace function
+create_pxt_session(p_web_user_id in numeric, p_expires in numeric, p_value in varchar)
+returns numeric as $$
+declare
+	l_id numeric;
+begin
+l_id := nextval( 'pxt_id_seq' );
+
+perform pg_dblink_exec(
+'insert into PXTSessions (id, value, expires, web_user_id) values (' ||
+l_id || ', ' || coalesce(quote_literal(p_value), 'NULL') ||
+', ' || p_expires || ', ' || coalesce(quote_literal(p_web_user_id), 'NULL') || '); commit');
+
+	return l_id;
+end;
+$$ language plpgsql;
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Script contents not properly displayed

2012-08-14 Thread Johannes Renner
Hello,

we found out that script contents are not properly displayed in the web UI
in Systems - Events - History (it's a perl page ...).

To reproduce:

- choose a system and go to Remote Command
- type in some script contents, e.g. a b c d e f g h
- schedule the command
- go to Events and click on the one just scheduled
- in the Details you will see (for the example above):

x23212f62696e2f73680a0a61206220632064206520662020672068

which is the ASCII code for


#!/bin/sh
a b c d e f g h


The attached patch fixes the problem by converting the ASCII codes into
a string using the pack() function.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From b00d36ff71783dd870e67cc969a9de2f55df4221 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 14 Aug 2012 11:46:43 +0200
Subject: [PATCH] Convert script contents from hex to string

---
 web/modules/rhn/RHN/Server.pm |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/web/modules/rhn/RHN/Server.pm b/web/modules/rhn/RHN/Server.pm
index 3022a09..6c12939 100644
--- a/web/modules/rhn/RHN/Server.pm
+++ b/web/modules/rhn/RHN/Server.pm
@@ -491,8 +491,9 @@ sub render {
   # sigh.  there has to be an easier way to filter out ansi control codes...
   $output-{OUTPUT} =~ s{\x1B\[(\d+[ABCDG]|[suK]|2J|(\d+;)*\d+m|=\d+[hl])}{}gism;
 
+  my $script = pack((H2)*, (substr($action-script_script, 1) =~ m/../g));
   $ret-{server_event_details} .=
-sprintf(EOQ, $run_as, $action-script_timeout || 0, PXT::HTML-htmlify_text($action-script_script));
+sprintf(EOQ, $run_as, $action-script_timeout || 0, PXT::HTML-htmlify_text($script));
 br/br/
 Run as: strong%s/strongbr/
 Timeout: strong%d/strong secondsbr/br/
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Script contents not properly displayed

2012-08-14 Thread Johannes Renner
On 08/14/2012 03:36 PM, Stephen Herr wrote:
 Johannes,
 
 Could you ensure in /usr/share/pgsql/postgresql.conf that bytea_output = 
 'escape', and if not see if
 that corrects your problem?
 
 -Stephen

Thanks, setting this option fixes the problem on the postgres 9.1 database we 
are using.

We also set standard_conforming_strings = 'off', do you think it makes still 
sense in
combination with bytea_output = 'escape'?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Query not elaborated?

2012-08-13 Thread Johannes Renner
On 08/06/2012 05:59 PM, Tomas Lestach wrote:
 Hello Johannes,
 
 On Friday 03 of August 2012 12:44:11 Johannes Renner wrote:
 Hi,

 We found out that commit 16f30f97d657ab7019489dfac5a11f67afc2143e apparently
 broke the Errata page for custom channels:

 https://hostname/rhn/channels/manage/errata/ListRemove.do?cid=XXX

 Sorting (by clicking on the columns) as well as filtering the list is broken
 and it seems as if the query is not actually elaborated.
 
 You're right here. My fault.
 We need to fetch the attributes according to those we want to sort in the 
 original query - even if they get elaborated later on.
 

 So I am coming up with two patch proposals, since I am not really sure about
 how we are intended to solve the problem:

 - The first patch fixes it by calling elaborate() manually once
 
 I wouldn't call the elaborate method manually. Since the mechanism has worked 
 till now, I'd rather revert my original commit.
 
 spacewalk.git: 43fbb66782bebe62e5aa323bc5edf70276d2609b
 
 - The second patch is a more generic fix and calls elaborate() in
   BaseManager.java even if PageControl is null
 
 What is the bug you try to fix with the 2nd patch? Could you describe the 
 misbehavior on a concrete page?

The second patch is just a more generic alternative fix for the same bug:

It makes sure that elaborate() will be called even if PageControl is null. It 
fixes
the bug since PageControl is null in the case of the above page. Further I was 
just
wondering why would we want to return an unelaborated list in this case?

Since this is probably not how the original mechanism is supposed to work, 
could you
maybe please give a short explanation? Who should be calling elaborate() and 
when?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Query not elaborated?

2012-08-03 Thread Johannes Renner
Hi,

We found out that commit 16f30f97d657ab7019489dfac5a11f67afc2143e apparently
broke the Errata page for custom channels:

https://hostname/rhn/channels/manage/errata/ListRemove.do?cid=XXX

Sorting (by clicking on the columns) as well as filtering the list is broken
and it seems as if the query is not actually elaborated.

So I am coming up with two patch proposals, since I am not really sure about
how we are intended to solve the problem:

- The first patch fixes it by calling elaborate() manually once
- The second patch is a more generic fix and calls elaborate() in
  BaseManager.java even if PageControl is null

What do you think, can you actually reproduce the bug?
Why would we want to return an unelaborated list in case PageControl is null?

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 3a688e6570caa2ef2a4d0a20ffbf3f9237865efe Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 3 Aug 2012 12:13:49 +0200
Subject: [PATCH] Call elaborate manually

---
 .../redhat/rhn/manager/channel/ChannelManager.java |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
index da09b61..950edbe 100644
--- a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
+++ b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
@@ -1288,7 +1288,7 @@ public class ChannelManager extends BaseManager {
 DataResult dr = m.execute(params);
 Map elabParams = new HashMap();
 elabParams.put(user_id, user.getId());
-dr.setElaborationParams(elabParams);
+dr.elaborate(elabParams);
 return dr;
 }
 
-- 
1.7.7

From 72bbdde0170645907c240057bbd1ffede1b716d0 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 3 Aug 2012 12:37:53 +0200
Subject: [PATCH] Return an elaborated list even if PageControl is null

---
 .../src/com/redhat/rhn/manager/BaseManager.java|   28 ++-
 .../redhat/rhn/manager/channel/ChannelManager.java |4 +--
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/manager/BaseManager.java b/java/code/src/com/redhat/rhn/manager/BaseManager.java
index 0a21cab..95fb490 100644
--- a/java/code/src/com/redhat/rhn/manager/BaseManager.java
+++ b/java/code/src/com/redhat/rhn/manager/BaseManager.java
@@ -106,8 +106,7 @@ public abstract class BaseManager {
 }
 
 /**
- * Process the PageControl against the DataResult. Returns an
- * strongunelaborated/strong list if PageControl is null.
+ * Process the PageControl against the DataResult.
  * @param dr
  * @param elabParams Named parameters for the elaboration query.
  * @param pc Page Control boundary definition.
@@ -116,10 +115,6 @@ public abstract class BaseManager {
 protected static DataResult processPageControl(DataResult dr,
 PageControl pc,
 Map elabParams) {
-if (elabParams != null) {
-dr.setElaborationParams(elabParams);
-}
-
 if (pc != null) {
 dr.setFilter(pc.hasFilter());
 if (pc.hasFilter()) {
@@ -140,22 +135,20 @@ public abstract class BaseManager {
 // now use the PageControl to limit the list to the
 // selected region.
 dr = dr.subList(pc.getStart() - 1, pc.getEnd());
+}
 
-//elaborate the data result to get the detailed information.
-if (elabParams != null) {
-dr.elaborate(elabParams);
-}
+//elaborate the data result to get the detailed information.
+if (elabParams != null) {
+dr.elaborate(elabParams);
 }
 
 return dr;
-
 }
 
 /**
  * Process the ListControl against the DataResult. The method
  * does not limit the number of results, unlike PageControl, and
- * simply provides filtering. Returns an strongunelaborated/strong
- * list if ListControl is null.
+ * simply provides filtering.
  * @param dr
  * @param elabParams Named parameters for the elaboration query.
  * @param lc ListControl filtering definition.
@@ -164,10 +157,6 @@ public abstract class BaseManager {
 protected static DataResult processListControl(DataResult dr,
 ListControl lc,
 Map elabParams) {
-if (elabParams != null) {
-dr.setElaborationParams(elabParams);
-}
-
 if (lc != null) {
 dr.setFilter(lc.hasFilter());
 if (lc.hasFilter()) {
@@ -184,12 +173,13 @@ public abstract class BaseManager {
 dr.setIndex(lc.createIndex(dr

[Spacewalk-devel] Timezone problem with DB timestamps

2012-07-30 Thread Johannes Renner
Hi,

We experienced a problem with rpm metadata generation: the taskomatic generated
build times of packages are different from the actual build times that are 
stored
in the rpm package. You should experience that problem as well.

We are wondering if it can be generally assumed that all of the values written 
to
date fields in the Database are converted to UTC beforehands? In this case we 
might
want to fix CachedStatement.java (method getObject()), so it will not put the 
local
timezone as default for all Date objects created using rs.getTimestamp() (only 
in
case the DB does not provide info about the timezone!).

A generic fix could maybe look like the attached patch, see here for javadoc of
the proposed method:

http://docs.oracle.com/javase/6/docs/api/java/sql/ResultSet.html#getTimestamp%28java.lang.String,%20java.util.Calendar%29

Otherwise we could also fix the package build times only, and do so in
PackageDto.setBuildTime(). What do you think?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 861cf5c634c611f19ed509fb96126adb5f4cea19 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 30 Jul 2012 17:12:06 +0200
Subject: [PATCH] Construct GMT millisecond value for timestamp if DB does not
 store timezone

---
 .../rhn/common/db/datasource/CachedStatement.java  |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/CachedStatement.java b/java/code/src/com/redhat/rhn/common/db/datasource/CachedStatement.java
index ab472a0..7133ec1 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/CachedStatement.java
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/CachedStatement.java
@@ -37,6 +37,7 @@ import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Calendar;
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashMap;
@@ -45,6 +46,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.StringTokenizer;
+import java.util.TimeZone;
 
 /**
  * A cached set of query/elaborator strings and the parameterMap hash maps.
@@ -772,7 +774,8 @@ public class CachedStatement {
 // this: August 23, 2005 12:00:00 AM PDT
 // vs the real date: August 23, 2005 1:36:12 PM PDT
 if (columnValue instanceof Date) {
-return rs.getTimestamp(columnName);
+Calendar cal = Calendar.getInstance(TimeZone.getTimeZone(GMT));
+return rs.getTimestamp(columnName, cal);
 }
 else if (columnValue instanceof BigDecimal) {
 return rs.getLong(columnName);
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-06-06 Thread Johannes Renner
On 06/04/2012 08:47 PM, Miroslav Suchy wrote:
 So here is the latest version of the patches.
 
 Committed (with some white space and check style fixes). Thanks for 
 contribution.

Thank you very much, here is already a first follow up patch:

Replace default string with the new constant RhnHelper.DEFAULT_FORWARD,
just for being consistent with recent changes here.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From d3fe367991d13c85b83e5a821688d4c32db5f248 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 6 Jun 2012 15:27:35 +0200
Subject: [PATCH] Refactor default to RhnHelper.DEFAULT_FORWARD

---
 .../images/ScheduleImageDeploymentAction.java  |3 ++-
 .../action/user/UserCredentialsDeleteAction.java   |3 ++-
 .../action/user/UserCredentialsEditAction.java |3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/systems/images/ScheduleImageDeploymentAction.java b/java/code/src/com/redhat/rhn/frontend/action/systems/images/ScheduleImageDeploymentAction.java
index 117f949..a3a9491 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/systems/images/ScheduleImageDeploymentAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/systems/images/ScheduleImageDeploymentAction.java
@@ -34,6 +34,7 @@ import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.action.renderers.ImagesRenderer;
 import com.redhat.rhn.frontend.struts.RequestContext;
 import com.redhat.rhn.frontend.struts.RhnAction;
+import com.redhat.rhn.frontend.struts.RhnHelper;
 import com.redhat.rhn.frontend.taglibs.list.ListTagHelper;
 import com.redhat.rhn.manager.action.ActionManager;
 import com.redhat.rhn.manager.system.SystemManager;
@@ -115,7 +116,7 @@ public class ScheduleImageDeploymentAction extends RhnAction {
 request.getRequestURI());
 }
 // Find the default destination
-forward = actionMapping.findForward(default);
+forward = actionMapping.findForward(RhnHelper.DEFAULT_FORWARD);
 }
 return forward;
 }
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsDeleteAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsDeleteAction.java
index 7dcacef..431a2f1 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsDeleteAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsDeleteAction.java
@@ -28,6 +28,7 @@ import com.redhat.rhn.domain.credentials.CredentialsFactory;
 import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.struts.RequestContext;
 import com.redhat.rhn.frontend.struts.RhnAction;
+import com.redhat.rhn.frontend.struts.RhnHelper;
 
 /**
  * Delete credentials for external systems or APIs.
@@ -61,6 +62,6 @@ public class UserCredentialsDeleteAction extends RhnAction {
 getStrutsDelegate().saveMessages(request, messages);
 return mapping.findForward(success);
 }
-return mapping.findForward(default);
+return mapping.findForward(RhnHelper.DEFAULT_FORWARD);
 }
 }
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
index 6473985..8048eb1 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserCredentialsEditAction.java
@@ -29,6 +29,7 @@ import com.redhat.rhn.domain.credentials.CredentialsFactory;
 import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.struts.RequestContext;
 import com.redhat.rhn.frontend.struts.RhnAction;
+import com.redhat.rhn.frontend.struts.RhnHelper;
 
 /**
  * Create and edit credentials for external systems or APIs.
@@ -93,6 +94,6 @@ public class UserCredentialsEditAction extends RhnAction {
 request.setAttribute(ATTRIB_CREDS, newCreds);
 }
 }
-return mapping.findForward(default);
+return mapping.findForward(RhnHelper.DEFAULT_FORWARD);
 }
 }
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] spacewalk-search declared as GPL-2.0 but contains Apache licenses

2012-05-31 Thread Johannes Renner
Hello,

the 'spacewalk-search' package is (according to spacewalk-search.spec) licensed 
as GPLv2,
but apparently it contains numerous Apache licenses throughout the files listed 
below.

The GPL-2.0 and the Apache licenses are not compatible with each other, 
according to the
Free Software Foundation: http://www.gnu.org/licenses/license-list.html#apache2.
This does most likely not pose a problem for the package, as the files appear 
to be mostly
content as opposed to code. Could you still please confirm that the Apache 
licensed files
are not actually being linked with GPL files?

I guess you would have to add both license texts to the package and it should 
be reflected
in the specfile (spacewalk-search.spec) as well:

License: GPLv2 -- License: GPL-2.0 and Apache-2.0

Thank you,
Johannes Renner

Please see the list of files here:

spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/hadoop
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/hadoop-config.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/hadoop-daemon.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/hadoop-daemons.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/rcc
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/slaves.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/start-all.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/start-balancer.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/start-dfs.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/start-mapred.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/stop-all.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/stop-balancer.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/stop-dfs.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/hadoop-0.18.1-core/bin/stop-mapred.sh
spacewalk-search-git-0.fd73b43/buildconf/tempjars/ibatis-2.3.0.677/com/ibatis/sqlmap/engine/builder/xml/sql-map-2.dtd
spacewalk-search-git-0.fd73b43/buildconf/tempjars/ibatis-2.3.0.677/com/ibatis/sqlmap/engine/builder/xml/sql-map-config-2.dtd
spacewalk-search-git-0.fd73b43/buildconf/tempjars/ibatis-2.3.0.677/license.txt
spacewalk-search-git-0.fd73b43/buildconf/tempjars/lucene-analyzers-2.3.2/META-INF/LICENSE.txt
spacewalk-search-git-0.fd73b43/buildconf/tempjars/lucene-core-2.3.2/META-INF/LICENSE.txt
spacewalk-search-git-0.fd73b43/buildconf/tempjars/nutch-2008-12-01_04-01-21/nutch-default.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/automaton-urlfilter.txt
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/common-terms.utf8
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/configuration.xsl
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/context.xsl
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/crawl-tool.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/crawl-urlfilter.txt
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/domain-suffixes.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/domain-suffixes.xsd
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/nutch-default.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/parse-plugins.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/prefix-urlfilter.txt
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/regex-normalize.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/regex-urlfilter.txt
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/subcollections.xml
spacewalk-search-git-0.fd73b43/nutch/crawl_jsp/conf/tika-mimetypes.xml
spacewalk-search-git-0.fd73b43/scripts/lukeall-0.8.1/META-INF/LICENSE.txt
spacewalk-search-git-0.fd73b43/scripts/lukeall-0.8.1/xml/about.xml
spacewalk-search-git-0.fd73b43/task-lib/commons-beanutils-1.7.0/META-INF/LICENSE.txt
spacewalk-search-git-0.fd73b43/task-lib/commons-logging-1.0.4/META-INF/LICENSE.txt
spacewalk-search-git-0.fd73b43/task-lib/commons-logging-1.0.4/org/apache/commons/logging/impl/package.html
spacewalk-search-git-0.fd73b43/task-lib/commons-logging-1.0.4/org/apache/commons/logging/package.html

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-05-21 Thread Johannes Renner
On 05/21/2012 02:17 PM, Miroslav Suchý wrote:
 On 05/18/2012 05:11 PM, Miroslav Suchy wrote:

 I still have to test the client part.
 
 in spec:
 Requires: python-curl
 
 In Fedora/RHEL the package is called python-pycurl, please fix it using %if

No problem.

 After image is scheduled, you land on:
 /rhn/systems/details/virtualization/VirtualGuestsList.do
 I would prefer to stay on:
 /rhn/systems/details/virtualization/Images.do

Oh sure, I agree with you, would prefer that as well.

 Beside that I do not see another problem.
 So please fix that and I will be happy to commit those patches.

Sounds good, I will fix all the things you mentioned, and will then send a new
version of the patches these days.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-05-18 Thread Johannes Renner
On 05/18/2012 02:45 PM, Miroslav Suchy wrote:
 In patch 0001 - schema db - you are missing upgrades for table 
 suseCredentialsType.
 
 And upgrades for table suseCredentials differs from clean install.

Oh yeah, sorry, I forgot to revise the upgrade files. I can come up with another
patch fixing that, or maybe rather with a new version of 0001.

But in which way it differs from clean install in this case?

Also at what index those files should start, looks like you are currently at
0047-rhnOrgQuota.sql?

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-05-16 Thread Johannes Renner
On 05/15/2012 09:29 PM, Miroslav Suchy wrote:
 On 15.5.2012 14:28, Johannes Renner wrote:
 On 05/15/2012 01:25 PM, Miroslav Suchy wrote:
 +Requires: susestudio-java-client

 This will require susestudio-java-client rpm? It is not present in neither 
 in Fedora nor in
 jpackage.
 Can I take:

 https://build.opensuse.org/package/view_file?file=susestudio-java-client.specpackage=susestudio-java-clientproject=Java%3Abaserev=3acd29dcce3b7fe9fad7c42e11358e75


 or you have some recent spec?

 I just checked it again: yes, please take it from there. This is the most 
 recent specfile
 and sources. In case you don't have the other required package in Fedora or 
 jpackage
 (simple-xml), you can get it from here as well:

 https://build.opensuse.org/package/show?package=simple-xmlproject=Java%3Abase

 Thanks,
 Johannes

 
 OK, I started with simple-xml. It sucessfully build on all supported 
 platforms. Thats good.
 
 Can you fix these warnings and errors of rpmlint:

Ok, I committed some changes to the spec, please see my comments below + the 
changes itself:

https://build.opensuse.org/package/rdiff?linkrev=basepackage=simple-xmlproject=Java%3Abaserev=3

 simple-xml.src: E: description-line-too-long C Simple is a high performance 
 XML serialization and
 configuration framework for Java.

I shortened the description lines to  79 characters.

 simple-xml.src: W: non-standard-group Development/Libraries/Java

Please assign whatever group is valid for Fedora/RedHat, since 
Development/Libraries/Java is
just the right group to be used for SUSE.

I guess we could have some %if (0%?rhel/?suse_version) %else ... though, if 
you prefer that.

 simple-xml.src: E: no-changelogname-tag
  this one can be ignored as first build with tito will fix it.

Good, since we usually maintain those changelogs in separate .changes files.

 simple-xml.src: W: invalid-license Apache-2.0

Same as above: please edit it and set the Fedora equivalent for your needs. If 
I change it, OBS
will print a warning, since Apache-2.0 is the correct license string for SUSE.

According to http://fedoraproject.org/wiki/Licensing you might have to put ASL 
2.0.

 simple-xml.src:53: W: setup-not-quiet

No idea about that one, OBS doesn't print that warning. I added -q to %setup, 
HTH.

 simple-xml.src: W: invalid-url Source0: simple-xml-2.6.2.zip

Fixed that and put a URL. Not a valid one though, since download URLs are 
dynamically generated
for this project that is hosted on sourceforge.

 And I see that upstream has new version: 2.6.3. Can you rebase to it? Is just 
 version bump
 sufficient or spec needs more changes?

The new version seems to build just fine, so I rebased the package.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-05-16 Thread Johannes Renner
On 05/16/2012 12:49 PM, Miroslav Suchy wrote:
 On 16.5.2012 11:01, Johannes Renner wrote:
 I guess we could have some %if (0%?rhel/?suse_version) %else ... though, 
 if you prefer that.
 
 Ok, I will put there %if and for license as well.

Good, let me see how that looks like and I will change add it to our package as 
well.

   simple-xml.src: W: invalid-url Source0: simple-xml-2.6.2.zip
 Fixed that and put a URL. Not a valid one though, since download URLs are 
 dynamically generated
 for this project that is hosted on sourceforge.
 
  http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net
 
 In this case valid url is:
 http://downloads.sourceforge.net/simple/simple-xml-2.6.3.zip
 
 so in spec should be:
 
 http://downloads.sourceforge.net/simple/%{name}-%{version}.zip

Thanks, I didn't know that. Changed it in our specfile as well.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-05-15 Thread Johannes Renner
On 05/15/2012 01:25 PM, Miroslav Suchy wrote:
 +Requires: susestudio-java-client
 
 This will require susestudio-java-client rpm? It is not present in neither in 
 Fedora nor in jpackage.
 Can I take:
 
 https://build.opensuse.org/package/view_file?file=susestudio-java-client.specpackage=susestudio-java-clientproject=Java%3Abaserev=3acd29dcce3b7fe9fad7c42e11358e75
 
 or you have some recent spec?

I just checked it again: yes, please take it from there. This is the most 
recent specfile
and sources. In case you don't have the other required package in Fedora or 
jpackage
(simple-xml), you can get it from here as well:

https://build.opensuse.org/package/show?package=simple-xmlproject=Java%3Abase

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Fix errata clone name generation in perl code

2012-04-23 Thread Johannes Renner
Hello,

Attached, please find the new algorithm for name generation of cloned errata,
written in Perl!

In the long run I guess we should definitely rather rewrite those pages in
Java, instead of maintaining both of the code stacks. Was there a specific
reason for the errata cloning page, why it was not rewritten? Or is it just
remaining by accident?

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From b497b86fdfa6054c60c35a2d78f5eb8a05a445af Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 23 Apr 2012 14:15:01 +0200
Subject: [PATCH] Fix errata clone name generation in perl code

---
 web/modules/rhn/RHN/DB/ErrataEditor.pm |   68 
 .../sniglets/Sniglets/ListView/ErrataList.pm   |2 +-
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/web/modules/rhn/RHN/DB/ErrataEditor.pm b/web/modules/rhn/RHN/DB/ErrataEditor.pm
index 249ef8d..d175a75 100644
--- a/web/modules/rhn/RHN/DB/ErrataEditor.pm
+++ b/web/modules/rhn/RHN/DB/ErrataEditor.pm
@@ -153,24 +153,47 @@ EOQ
   return ($new_eid);
 }
 
+# This duplicates the algorithm in PublishErrataHelper.java
 sub find_next_advisory {
   my $adv = shift || '';
   my $adv_name = shift || '';
-  my $suffix = '';
-  my $i = 1;
+  my $eid = shift;
 
-  $adv = 'CL' . substr($adv, 2);
-  $adv_name = 'CL' . substr($adv_name, 2);
+  # Set adv equal to adv_name if unset
+  unless ($adv) {
+$adv = $adv_name;
+  }
 
-  if (advisory_exists($adv) || advisory_name_exists($adv_name)) {
-$suffix = sprintf(-%u, $i++);
-$adv = $adv . $suffix;
-$adv_name = $adv_name . $suffix;
+  if (not erratum_is_clone($eid)) {
+# For RH errata, replace 'RH' with 'CL-', else prepend
+if ('RH' eq substr($adv_name, 0, 2)) {
+  $adv = 'CL-' . substr($adv, 2);
+  $adv_name = 'CL-' . substr($adv_name, 2);
+} else {
+  $adv = 'CL-' . $adv;
+  $adv_name = 'CL-' . $adv_name;
+}
+  } else {
+# Erratum is a clone, prepend 'CL-', if there is no prefix yet
+if ('-' ne substr($adv, 2, 1) || '-' ne substr($adv_name, 2, 1)) {
+  $adv = 'CL-' . $adv;
+  $adv_name = 'CL-' . $adv_name;
+}
+  }
 
-while (advisory_exists($adv) || advisory_name_exists($adv_name)) {
-  substr($adv, -1, 1) = $i;
-  substr($adv_name, -1, 1) = $i;
-  $i++;
+  # Find the next valid advisory name that doesn't exist yet
+  while (advisory_exists($adv) || advisory_name_exists($adv_name)) {
+my $c1 = substr($adv, 1, 1);
+if ('Z' eq $c1) {
+  # Get the next c0
+  my $c0next = ++substr($adv, 0, 1);
+  $adv = $c0next . 'A' . substr($adv, 2);
+  $adv_name = $c0next . 'A' . substr($adv_name, 2);
+} else {
+  # Get the next c1
+  my $c1next = ++$c1;
+  $adv = substr($adv, 0, 1) . $c1next . substr($adv, 2);
+  $adv_name = substr($adv_name, 0, 1) . $c1next . substr($adv_name, 2);
 }
   }
 
@@ -200,7 +223,7 @@ sub clone_into_org {
   my $adv = $new-advisory;
   my $adv_name = $new-advisory_name;
 
-  ($adv, $adv_name) = find_next_advisory($adv, $adv_name);
+  ($adv, $adv_name) = find_next_advisory($adv, $adv_name, $old_eid);
 
   $new-advisory($adv);
   $new-advisory_name($adv_name);
@@ -272,6 +295,25 @@ EOQ
   return ($new_eid);
 }
 
+sub erratum_is_clone {
+  my $eid = shift;
+  my $dbh = RHN::DB-connect;
+
+  my $query =EOQ;
+SELECT 1
+  FROM dual
+ WHERE EXISTS (SELECT 1 FROM rhnErrataCloned E WHERE E.id = :eid)
+OR EXISTS (SELECT 1 FROM rhnErrataClonedTmp ET WHERE ET.id = :eid)
+EOQ
+
+  my $sth = $dbh-prepare($query);
+  $sth-execute_h(eid = $eid);
+  my ($exists) = $sth-fetchrow;
+  $sth-finish;
+
+  return ($exists ? 1 : 0);
+}
+
 sub advisory_exists {
   my $adv = shift;
 
diff --git a/web/modules/sniglets/Sniglets/ListView/ErrataList.pm b/web/modules/sniglets/Sniglets/ListView/ErrataList.pm
index 685dce6..b3af014 100644
--- a/web/modules/sniglets/Sniglets/ListView/ErrataList.pm
+++ b/web/modules/sniglets/Sniglets/ListView/ErrataList.pm
@@ -361,7 +361,7 @@ sub potential_for_cloned_channel_provider {
   $row-{OWNED_ERRATA_LIST} = '(none)';
 }
 
-my ($adv, $adv_name) = RHN::DB::ErrataEditor::find_next_advisory($row-{ADVISORY}, $row-{ADVISORY_NAME});
+my ($adv, $adv_name) = RHN::DB::ErrataEditor::find_next_advisory($row-{ADVISORY}, $row-{ADVISORY_NAME}, $row-{ID});
 
 push(@options, ( { label = Clone as ${adv_name},
 		   value = 'clone_new' },
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Naming of cloned errata

2012-04-13 Thread Johannes Renner
On 04/11/2012 02:28 PM, Jan Pazdziora wrote:
 On Wed, Apr 11, 2012 at 11:43:07AM +0200, Johannes Renner wrote:

 I agree that clones of clones should get CM, CN, etc. instead of 
 CL-CL-kernel*.
 How would you propose to detect clones for implementing this algorithm?
 
 How about having a new database table with prefixes and replacements,
 where the prefix could be empty (NULL) to mean prepend. Record with
 the longest matching prefix would provide the replacement string.
 
 This way you'd be able to have sane defaults in Spacewalk and every
 downstream product could customize this mapping table to meet their
 business requirements.

Did I get that right, the table for RH would look like this:

Prefix | Replacement

RH | CL
CL | CM
CM | CN
CN | CO
...| ...

While for us it would look like this:

NULL | CL-
CL | CM
CM | CN
CN | CO
...| ...

When cloning, we wouldn't check if we actually have a clone already,
but just lookup the table for the longest match and replace?

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] Removed strings still being used

2012-04-02 Thread Johannes Renner
Hey,

After running our testsuite against a recent codebase, we saw (from 
catalina.out)
that the following removed strings seem to be still in use:

configchannelfilter.name
configfilefilter.path
config_subscribed_systems.unsubscribeSystems.success
config_target_systems.subscribeSystems.success
help.jsp.quickstart.detailed
help.jsp.quickstart.title
preferences.critical-probes.description
preferences.critical-probes.name
preferences.critical-systems.description
preferences.critical-systems.name
preferences.inactive-systems.description
preferences.inactive-systems.name
sdc.config.differing.files_1
sdc.config.diff.files_1_dirs_0_symlinks_0
userlist.jsp.disabled
userlist.jsp.enabled

It looks like these are all from the .../jsp/StringResource_en_US.xml file.
So, how to cleanly revert only parts of such a big commit? I saw you did that
already for e.g. doing 8899861.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Checkstyle 4.1 vs. 4.3

2012-03-30 Thread Johannes Renner
Hey,

You are using checkstyle 4.1 it seems, while we use version 4.3.
Our version seems to need the lib directory jars to be contained
in the classpath for finding exception classes the are referenced
by @throws clauses. Otherwise we get a lot of these errors:

./code/src/com/redhat/rhn/common/hibernate/HibernateFactory.java:0: Got an 
exception -
java.lang.RuntimeException: Unable to get class information for @throws tag 
'HibernateException'.

Also 4.3 doesn't like the @{inherit-doc} for methods that are not
actually overrides, like the constructor in ScapActionDetails.java:

./code/src/com/redhat/rhn/domain/action/scap/ScapActionDetails.java:33:5: 
Invalid use of the
{@inheritDoc} tag.

Just in case you are planning to upgrade or want to fix anyways.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 0bcacfc4bbc032c17263198f5bc8dde4ec000aef Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Fri, 30 Mar 2012 10:26:25 +0200
Subject: [PATCH] Add lib directory to checkstyle classpath

---
 java/spacewalk-java.spec |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/spacewalk-java.spec b/java/spacewalk-java.spec
index 2fc7361..7ae7c45 100644
--- a/java/spacewalk-java.spec
+++ b/java/spacewalk-java.spec
@@ -327,7 +327,7 @@ ant -Dprefix=$RPM_BUILD_ROOT init-install compile
 
 %if 0%{?run_checkstyle}
 echo Running checkstyle on java main sources
-export CLASSPATH=build/classes
+export CLASSPATH=build/classes:build/build-lib/*
 export BASE_OPTIONS=-Djavadoc.method.scope=public \
 -Djavadoc.type.scope=package \
 -Djavadoc.var.scope=package \
-- 
1.7.7

From 2b57a83dd3346dcaeff55586cacc13e5728e3d38 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 29 Mar 2012 16:10:13 +0200
Subject: [PATCH] Fix checkstyle error (invalid use of the {@inheritDoc} tag)

---
 .../rhn/domain/action/scap/ScapActionDetails.java  |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/action/scap/ScapActionDetails.java b/java/code/src/com/redhat/rhn/domain/action/scap/ScapActionDetails.java
index 7c34c49..885da51 100644
--- a/java/code/src/com/redhat/rhn/domain/action/scap/ScapActionDetails.java
+++ b/java/code/src/com/redhat/rhn/domain/action/scap/ScapActionDetails.java
@@ -28,7 +28,7 @@ public class ScapActionDetails extends ActionChild {
 private byte[] parameters;
 
 /**
- * {@inheritDoc}
+ * Default constructor.
  */
 public ScapActionDetails() {
 super();
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Naming of cloned errata

2012-03-30 Thread Johannes Renner
On 03/07/2012 11:57 AM, Johannes Renner wrote:
 On 03/06/2012 05:42 PM, Johannes Renner wrote:
 Hey,

 I want to propose a change concerning the names of cloned errata.
 Currently, names of cloned errata are generated like this:

 String clonedAdvisoryName = CL + published.getAdvisoryName().substring(3);
 String clonedAdvisory = CL + published.getAdvisory().substring(3);

 The reason for doing it like this might have been a small sized field in the
 database, but AFAIK we already upstreamed a patch to enlarge the respective
 field. Since cloned errata currently end up with rather strange names, I
 would like to propose to do the naming like this instead:

 String clonedAdvisoryName = CL- + published.getAdvisoryName();
 String clonedAdvisory = CL- + published.getAdvisory();

 Even the classification used for the Red Hat patches (E, S and B) gets lost
 when removing the first 3 characters, since names start with RH(E|S|B). So
 please tell me if I am missing any reason to keep the old naming scheme.

 Attached please find my proposed patch for spacewalk master.
 
 Of course this algorithm is duplicated in the perl parts of Spacewalk ;-)
 Patch is attached.

Just reactivating/raising this thread on the list since there is no comments
yet, and I'm curious if you will accept the proposed changes or not.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] ivy

2012-03-28 Thread Johannes Renner
On 03/28/2012 10:38 AM, Tomas Lestach wrote:
 On Tuesday 27 of March 2012 23:00:53 Miroslav Suchy wrote:
 Do we still use ivy from jpp? Can we use apache-ivy from Fedora?
 
 Where do you see, we use ivy? I believe we do not require ivy in run or build 
 time.
 If you mean development workstation setup, then I'd say no. We use an old ivy 
 version from the really old 1.7 jpackage repo. I remember Simon hit some 
 troubles, when he wanted to use newer ivy from jpackage 5.0.
 
 Tomas

That's correct, ivy seems to be used only for setting up workspaces, but I would
nevertheless recommend to switch to a newer version.

Troubles are not very big I guess, it's mainly the taskdefs 
(build-taskdefs.xml):

fr.jayasoft.ivy.ant.* - org.apache.ivy.ant.*

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] Deployment of images built with SUSE Studio

2012-03-27 Thread Johannes Renner
Hello,

We recently added a feature to our branch of Spacewalk that enables users to
deploy images that were built with SUSE Studio [1] to registered virtual host
systems.

The feature involves a new action type to be put in table rhnActionType, which
we chose to give the ID 500 for now (just to make sure there will be no clash in
the near future). Would you maybe want to provide us with an ID for this action
that we can safely use throughout the future or what is the preferred procedure
here?

Some info about the feature itself:

Basically there is a new subtab to the virtualization tab on a specific system
for listing a user's images and scheduling image deployments based on the list
of images. Further there is another simple UI for entering a user's credentials
for querying the Studio API. If you are interested in this feature I will be
happy to prepare patches for spacewalk master, just tell me.

Thank you,
Johannes

[1] http://susestudio.com

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] Deployment of images built with SUSE Studio

2012-03-27 Thread Johannes Renner
On 03/27/2012 11:35 AM, Miroslav Suchý wrote:
 On 03/27/2012 10:27 AM, Johannes Renner wrote:
 The feature involves a new action type to be put in table rhnActionType, 
 which
 we chose to give the ID 500 for now (just to make sure there will be no 
 clash in
 the near future). Would you maybe want to provide us with an ID for this 
 action
 that we can safely use throughout the future or what is the preferred 
 procedure
 here?
 
 We are not going to assign ID for action which is not in our master.

Thanks for the clarification.

 If you ever decide to contribute your work to Spacewalk project, you can take 
 advantage to have it
 in upstream and no one will override that id.
 If you decide - for whatever reason - to keep it private, you will have to 
 live with the fear and
 uncertainty that one day we will use that id for something else.

As I have written in my first mail:

We would be willing to commit this feature upstream, but I am actually doubting
that you will just take the commits and push them into master.

Reasons may include:

- The whole feature is very SUSE specific (integration of a SUSE product)
- There is a new dependency on a library for talking to the API
- There is a new table for storing a user's API credentials (suseXYZ namespace)

I can prepare and send patches if you are fine with such things. I just need to
make sure that there is some interest and that these patches will not be 
rejected
after I put in the work to make them apply to Spacewalk master.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-03-08 Thread Johannes Renner
On 03/07/2012 10:04 AM, Tomas Lestach wrote:
 I will find time for that in the next days and I can then come up with a new
 patch for that file after I verified the behavior.
 
 Great!
 Thank you Johannes,

So, AFAICS the rhnServerNeededCache table is there for caching which errata need
to be applied to a system together with the respective package ids. So I don't
really see how it makes sense to insert rows in there with errata id == null.

I compared the resulting cache entries with what we get when running the stored
procedure 'rhn_server.update_needed_cache' (with respective parameters of 
course).
The output was equal, but missing the 'null' rows.

Even if i think these 'null' entries don't hurt anybody I would propose to apply
the attached patch to PublishErrataAction.java (also the code was quite a mess).

Please tell me if this breaks anything and throw away my patch in this case ;-)

Thanks,
Johannes

P.S.: Further, if this is all true, I would actually question the whole method
'insertCacheForChannelPackagesAsync()' in ErrataCacheManager.java, since it does
exactly this:

...
uece.setErrataId(null);
...

But it's called in 3-4 other places as well, so you (or somebody else who knows)
should better have a look and verify this first, see the second patch.

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 22cab15a789c1d8cc50f2efb8244fbec1c9a2236 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 8 Mar 2012 15:26:33 +0100
Subject: [PATCH] Remove obsolete cache insertion

---
 .../action/channel/manage/PublishErrataAction.java |   17 ++---
 1 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
index 7a24554..8d98e3d 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
@@ -22,7 +22,6 @@ import com.redhat.rhn.frontend.struts.RequestContext;
 import com.redhat.rhn.frontend.struts.RhnListAction;
 import com.redhat.rhn.manager.channel.ChannelManager;
 import com.redhat.rhn.manager.errata.ErrataManager;
-import com.redhat.rhn.manager.errata.cache.ErrataCacheManager;
 import com.redhat.rhn.manager.rhnset.RhnSetDecl;
 
 import org.apache.log4j.Logger;
@@ -79,9 +78,7 @@ public class PublishErrataAction extends RhnListAction {
 
 ErrataManager.publishErrataToChannelAsync(currentChan, errataIds, user);
 
-//ErrataManager.publishErrataToChannel(currentChan, errataIds, user);
-
-
+// Add missing packages to the channel
 ListLong pidList = new ArrayListLong();
 pidList.addAll(packageIds);
 
@@ -93,28 +90,18 @@ public class PublishErrataAction extends RhnListAction {
 }
 }
 
-
-//update the errata info
-List chanList = new ArrayList();
-chanList.add(currentChan.getId());
-ErrataCacheManager.insertCacheForChannelPackagesAsync(chanList, pidList);
 ChannelManager.refreshWithNewestPackages(currentChan, web.errata_push);
 request.setAttribute(cid, cid);
 
+// Add success message
 ActionMessages msg = new ActionMessages();
 String[] params = {errataIds.size() + , packageIds.size() + ,
 currentChan.getName()};
 msg.add(ActionMessages.GLOBAL_MESSAGE,
 new ActionMessage(frontend.actions.channels.manager.add.success,
 params));
-
 getStrutsDelegate().saveMessages(requestContext.getRequest(), msg);
 
 return mapping.findForward(default);
 }
-
-
-
-
-
 }
-- 
1.7.7

From dba13014a02468031faeab8700d0959f006946c3 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 8 Mar 2012 15:41:16 +0100
Subject: [PATCH] Remove obsolete cache insertion completely

---
 .../manage/ChannelPackagesAddConfirmAction.java|1 -
 .../action/errata/RemovePackagesAction.java|7 --
 .../channel/software/ChannelSoftwareHandler.java   |8 ---
 .../manager/errata/cache/ErrataCacheManager.java   |   21 
 4 files changed, 0 insertions(+), 37 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddConfirmAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddConfirmAction.java
index f98206a..77f69e7 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddConfirmAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddConfirmAction.java
@@ -125,7 +125,6 @@ public class ChannelPackagesAddConfirmAction extends RhnAction {
 ListLong packList = new ArrayListLong();
 chanList.add(chan.getId

Re: [Spacewalk-devel] [PATCH] Naming of cloned errata

2012-03-07 Thread Johannes Renner
On 03/06/2012 05:42 PM, Johannes Renner wrote:
 Hey,
 
 I want to propose a change concerning the names of cloned errata.
 Currently, names of cloned errata are generated like this:
 
 String clonedAdvisoryName = CL + published.getAdvisoryName().substring(3);
 String clonedAdvisory = CL + published.getAdvisory().substring(3);
 
 The reason for doing it like this might have been a small sized field in the
 database, but AFAIK we already upstreamed a patch to enlarge the respective
 field. Since cloned errata currently end up with rather strange names, I
 would like to propose to do the naming like this instead:
 
 String clonedAdvisoryName = CL- + published.getAdvisoryName();
 String clonedAdvisory = CL- + published.getAdvisory();
 
 Even the classification used for the Red Hat patches (E, S and B) gets lost
 when removing the first 3 characters, since names start with RH(E|S|B). So
 please tell me if I am missing any reason to keep the old naming scheme.
 
 Attached please find my proposed patch for spacewalk master.

Of course this algorithm is duplicated in the perl parts of Spacewalk ;-)
Patch is attached.

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 1397508621edc9e7a4804c3aee46a74a23421bb4 Mon Sep 17 00:00:00 2001
From: Michael Calmer m...@suse.de
Date: Wed, 7 Mar 2012 11:13:43 +0100
Subject: [PATCH] Fix naming of cloned patches to not remove the first 3 chars

---
 web/modules/rhn/RHN/DB/ErrataEditor.pm |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/web/modules/rhn/RHN/DB/ErrataEditor.pm b/web/modules/rhn/RHN/DB/ErrataEditor.pm
index 8c19fa5..249ef8d 100644
--- a/web/modules/rhn/RHN/DB/ErrataEditor.pm
+++ b/web/modules/rhn/RHN/DB/ErrataEditor.pm
@@ -159,8 +159,8 @@ sub find_next_advisory {
   my $suffix = '';
   my $i = 1;
 
-  $adv = 'CL' . substr($adv, 3);
-  $adv_name = 'CL' . substr($adv_name, 3);
+  $adv = 'CL-' . $adv;
+  $adv_name = 'CL-' . $adv_name;
 
   if (advisory_exists($adv) || advisory_name_exists($adv_name)) {
 $suffix = sprintf(-%u, $i++);
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Naming of cloned errata

2012-03-06 Thread Johannes Renner
Hey,

I want to propose a change concerning the names of cloned errata.
Currently, names of cloned errata are generated like this:

String clonedAdvisoryName = CL + published.getAdvisoryName().substring(3);
String clonedAdvisory = CL + published.getAdvisory().substring(3);

The reason for doing it like this might have been a small sized field in the
database, but AFAIK we already upstreamed a patch to enlarge the respective
field. Since cloned errata currently end up with rather strange names, I
would like to propose to do the naming like this instead:

String clonedAdvisoryName = CL- + published.getAdvisoryName();
String clonedAdvisory = CL- + published.getAdvisory();

Even the classification used for the Red Hat patches (E, S and B) gets lost
when removing the first 3 characters, since names start with RH(E|S|B). So
please tell me if I am missing any reason to keep the old naming scheme.

Attached please find my proposed patch for spacewalk master.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 30c638790f3f3d354c2381c6f9b2f23c90e20a50 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 6 Mar 2012 17:11:55 +0100
Subject: [PATCH] Fix naming of cloned patches to not remove the first 3 chars

---
 .../redhat/rhn/domain/errata/ErrataFactory.java|4 ++--
 .../rhn/domain/errata/test/ErrataFactoryTest.java  |4 ++--
 .../action/channel/manage/PublishErrataHelper.java |4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
index cda8c6a..6670b86 100644
--- a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
@@ -362,8 +362,8 @@ public class ErrataFactory extends HibernateFactory {
 public static Errata createClone(Org org, Errata e) {
 
 
-String baseClonedAdvisoryName = CL + e.getAdvisoryName().substring(3);
-String baseClonedAdvisory = CL + e.getAdvisory().substring(3);
+String baseClonedAdvisoryName = CL- + e.getAdvisoryName();
+String baseClonedAdvisory = CL- + e.getAdvisory();
 String clonedAdvisory = baseClonedAdvisory;
 String clonedAdvisoryName = baseClonedAdvisoryName;
 boolean unusedNameFound = false;
diff --git a/java/code/src/com/redhat/rhn/domain/errata/test/ErrataFactoryTest.java b/java/code/src/com/redhat/rhn/domain/errata/test/ErrataFactoryTest.java
index 601193e..89565df 100644
--- a/java/code/src/com/redhat/rhn/domain/errata/test/ErrataFactoryTest.java
+++ b/java/code/src/com/redhat/rhn/domain/errata/test/ErrataFactoryTest.java
@@ -316,8 +316,8 @@ public class ErrataFactoryTest extends BaseTestCaseWithUser {
Channel baseChannel = ChannelTestUtils.createBaseChannel(user);
published.addChannel(baseChannel);
Errata clone = ErrataFactory.createClone(user.getOrg(), published);
-   String clonedAdvisoryName = CL + published.getAdvisoryName().substring(3);
-   String clonedAdvisory = CL + published.getAdvisory().substring(3);
+   String clonedAdvisoryName = CL- + published.getAdvisoryName();
+   String clonedAdvisory = CL- + published.getAdvisory();
 
assertNotNull(clone.getId());
assertFalse(published.isCloned());
diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataHelper.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataHelper.java
index c3c7127..c28a6bf 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataHelper.java
@@ -107,8 +107,8 @@ public class PublishErrataHelper {
 }
 
 
-String baseClonedAdvisoryName = CL + original.getAdvisoryName().substring(3);
-String baseClonedAdvisory = CL + original.getAdvisory().substring(3);
+String baseClonedAdvisoryName = CL- + original.getAdvisoryName();
+String baseClonedAdvisory = CL- + original.getAdvisory();
 String clonedAdvisory = baseClonedAdvisory;
 String clonedAdvisoryName = baseClonedAdvisoryName;
 boolean unusedNameFound = false;
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-03-05 Thread Johannes Renner
On 03/05/2012 03:40 PM, Tomas Lestach wrote:
 On Friday 02 of March 2012 18:11:41 Johannes Renner wrote:
 On 02/29/2012 05:38 PM, Cliff Perry wrote:
 On 02/29/2012 11:05 AM, Johannes Renner wrote:
 Hello,

 I am investigating into a bug about cloned channels and errata. This
 is how to reproduce it on the UI:

 1. Clone a channel that contains errata, but select clone without
 errata 2. Go to Channels - Manage Software Channels - Errata -
 Add - Add Custom Errata 
(or Add RedHat Errata) - Unmark the checkbox Package Assoc.
if necessary 
 3. Choose an Erratum, clone it and confirm

 Result: The cloned erratum can be found in the cloned channel as
 CLxxx

 5. Now subscribe a system to the newly cloned channel only
 6. Go to this System - Software - Errata

 Result: There is two errata listed, xxx and CLxxx, but only
 CLxxx should be. Also the table 'RhnServerNeededCache' shows even
 more entries where errata_id == null?

 I found a lot of open bugs about this topic, but not exactly this one.
 If one of you can reproduce it I could create a bug report for
 spacewalk, or are you aware of such misbehavior already?

 To me it looks like the used statement
 ('insert_new_cache_entries_by_packages' in ErrataCache_queries.xml)
 is not doing the right thing. There even seems to be an easy fix that
 I attached as a patch for master, which just calls a stored procedure
 ('update_needed_cache_for_channel', ErrataCache_queries.xml) instead.

 Maybe someone of you who knows about this code could have a look at
 the issue, I might be missing something ..

 Replying to add more background. I was not aware of this specific bug
 myself either. I'd be interested to see if we can reproduce on Sat 5.4
 code as well as current Spacewalk code.

 This area of the code seems to get updates and re-factor once every
 couple of years, due to subtle bugs in logic, along with
 scalability/performance feedback.

 The particular line you attached to propose changing was last changed in
 Feb 2009 as part of a larger re-factor:
 http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=3707af41
 49d84a1ddb1ce693c78f7b791a044cd1

 If memory serves me correct, we were in the middle of a Satellite
 release cycle, so changing the stored procedure was not an ideal
 solution for Satellite. We fixed Java code - which performed well.

  I'm not sure if I'd want the fix, as proposed, in 1.7, without us

 spending time to confirm it is the best solution for logic and
 performance. I don't think we will have time to dedicate to assist until
 post 1.7. If one the guys finds we have time to dig in before 1.7 goes
 out in the next week (hopefully) - we will.

 Cliff

 Sorry, I now found out I was experiencing this bug and was missing the
 patch:

 https://bugzilla.redhat.com/show_bug.cgi?id=707658

 So the cloned patches now show up correctly on the UI, but anyways I think
 there is some code in here that does strange things. E.g. when I analyze the
 file PublishErrataAction.java:

 The newly cloned patch is published by calling in line 80:

 ErrataManager.publishErrataToChannelAsync(...)

 This also triggers insertion into the cache table (rhnServerNeededCache).
 So why do we need to call later on (and without the actual errata ids):

 ErrataCacheManager.insertCacheForChannelPackagesAsync(chanList, pidList);
 (line 100)

 This seems to insert another row into rhnServerNeededCache where the eid is
 null! If this is wanted behaviour, then what is it for?
 
 Right Johannes,
 
 good catch.
 this definitelly isn't wanted behavior.
 If the added packages to a channals are associated with any erratum (what is 
 our case), we definitelly want to consider also the erratum, when 
 regenerating 
 errata cache.
 
 Will you find some time to verify the behavior, when removing the errata 
 cache 
 update on line 100?
 If not, please, file a BZ for the issue.

I will find time for that in the next days and I can then come up with a new
patch for that file after I verified the behavior.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-02-29 Thread Johannes Renner
Hello,

I am investigating into a bug about cloned channels and errata. This is how to
reproduce it on the UI:

1. Clone a channel that contains errata, but select clone without errata
2. Go to Channels - Manage Software Channels - Errata - Add - Add Custom 
Errata
   (or Add RedHat Errata) - Unmark the checkbox Package Assoc. if necessary
3. Choose an Erratum, clone it and confirm

Result: The cloned erratum can be found in the cloned channel as CLxxx

5. Now subscribe a system to the newly cloned channel only
6. Go to this System - Software - Errata

Result: There is two errata listed, xxx and CLxxx, but only CLxxx should 
be.
Also the table 'RhnServerNeededCache' shows even more entries where errata_id 
== null?

I found a lot of open bugs about this topic, but not exactly this one. If one 
of you
can reproduce it I could create a bug report for spacewalk, or are you aware of 
such
misbehavior already?

To me it looks like the used statement ('insert_new_cache_entries_by_packages' 
in
ErrataCache_queries.xml) is not doing the right thing. There even seems to be an
easy fix that I attached as a patch for master, which just calls a stored 
procedure
('update_needed_cache_for_channel', ErrataCache_queries.xml) instead.

Maybe someone of you who knows about this code could have a look at the issue,
I might be missing something ..

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 25cac35e2f20c249dd18ba522fe112ab058acd08 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 29 Feb 2012 16:52:43 +0100
Subject: [PATCH] Fix bug about cloned channels and errata

---
 .../action/channel/manage/PublishErrataAction.java |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
index 7a24554..226bb8a 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/PublishErrataAction.java
@@ -97,7 +97,7 @@ public class PublishErrataAction extends RhnListAction {
 //update the errata info
 List chanList = new ArrayList();
 chanList.add(currentChan.getId());
-ErrataCacheManager.insertCacheForChannelPackagesAsync(chanList, pidList);
+ErrataCacheManager.updateCacheForChannelsAsync(chanList);
 ChannelManager.refreshWithNewestPackages(currentChan, web.errata_push);
 request.setAttribute(cid, cid);
 
-- 
1.7.7

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Bug about cloned channels and errata

2012-02-29 Thread Johannes Renner
On 02/29/2012 05:38 PM, Cliff Perry wrote:
 On 02/29/2012 11:05 AM, Johannes Renner wrote:
 Hello,

 I am investigating into a bug about cloned channels and errata. This is how 
 to
 reproduce it on the UI:

 1. Clone a channel that contains errata, but select clone without errata
 2. Go to Channels - Manage Software Channels - Errata - Add - Add Custom 
 Errata
(or Add RedHat Errata) - Unmark the checkbox Package Assoc. if 
 necessary
 3. Choose an Erratum, clone it and confirm

 Result: The cloned erratum can be found in the cloned channel as CLxxx

 5. Now subscribe a system to the newly cloned channel only
 6. Go to this System - Software - Errata

 Result: There is two errata listed, xxx and CLxxx, but only CLxxx 
 should be.
 Also the table 'RhnServerNeededCache' shows even more entries where 
 errata_id == null?

 I found a lot of open bugs about this topic, but not exactly this one. If 
 one of you
 can reproduce it I could create a bug report for spacewalk, or are you aware 
 of such
 misbehavior already?

 To me it looks like the used statement 
 ('insert_new_cache_entries_by_packages' in
 ErrataCache_queries.xml) is not doing the right thing. There even seems to 
 be an
 easy fix that I attached as a patch for master, which just calls a stored 
 procedure
 ('update_needed_cache_for_channel', ErrataCache_queries.xml) instead.

 Maybe someone of you who knows about this code could have a look at the 
 issue,
 I might be missing something ..

 
 Replying to add more background. I was not aware of this specific bug
 myself either. I'd be interested to see if we can reproduce on Sat 5.4
 code as well as current Spacewalk code.
 
 This area of the code seems to get updates and re-factor once every
 couple of years, due to subtle bugs in logic, along with
 scalability/performance feedback.
 
 The particular line you attached to propose changing was last changed in
 Feb 2009 as part of a larger re-factor:
 http://git.fedorahosted.org/git/?p=spacewalk.git;a=commitdiff;h=3707af4149d84a1ddb1ce693c78f7b791a044cd1
 
 If memory serves me correct, we were in the middle of a Satellite
 release cycle, so changing the stored procedure was not an ideal
 solution for Satellite. We fixed Java code - which performed well.
 
  I'm not sure if I'd want the fix, as proposed, in 1.7, without us
 spending time to confirm it is the best solution for logic and
 performance. I don't think we will have time to dedicate to assist until
 post 1.7. If one the guys finds we have time to dig in before 1.7 goes
 out in the next week (hopefully) - we will.
 
 Cliff

Thanks for your reply. The patch was not intended to go into 1.7 directly and 
it is
probably not even complete as it is. It rather reflects the smallest possible 
change
that seems to fix the bug. More code in the same class will be obsolete when we 
call
the stored procedure, e.g. we do not need the list of all packages of the 
current
channel anymore.

If one of you could have a look as soon as there is time, it would be great.
After 1.7 is fine of course.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Fix query to determine available config channels in SSM

2011-12-06 Thread Johannes Renner
On 12/05/2011 06:53 PM, Michael Mraka wrote:
 Hi Johannes,
 
 although Tomas commited your patch neither original version nor yours
 are correct.
 
 a) Select from rhnSet was not restricted to user so it might interfere
 with other users SSM servers.
 
 b) Two servers in SSM and three channels:
  server1 subscribed to config1 and config2
  server2 is subscribed only to config1
  (no server subscribed to config3)
 query returns only config3 while it should have return config2, config3. 
 
 So I went ahead and rewrote queries completely, please check whether
 they work for you. (Commits a0ac26914ef0c80202dc0d707966ec34f523053e and
 d50e60bc201b11c7b23c7f7eb06f6a58ee593fb0)

Yes, that's right, thanks for the investigation. I tested your new queries
and they seem to do the right thing now, finally.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Fix query to determine available config channels in SSM

2011-11-29 Thread Johannes Renner
Hello,

It looks like that the following query is not actually doing what it should do:

config_queries.xml: ssm_channels_for_subscribe_choose

How to see that: Put two systems in the SSM where only one of those is 
subscribed
to a certain config channel. This config channel should appear in the SSM as an
option to subscribe to, because not ALL of the systems in SSM are already 
subscribed.
What happens is, that as soon as one of the systems in SSM is subscribed to a 
certain
channel, this channel does not appear as an available config channel in SSM 
anymore.

I found that the attached patch fixes the issue: While replacing = with is 
is
obviously correct, I also found, that as soon as SCC.server_id is null (in the 
last
subquery) the SCC.config_channel_id is also null. Therefore the proposed fix.
Further, since the next query in that file is just the negation of this one, it 
would
probably also need a fix.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 8493f940c7b6fb27e4f25f211865890fcf71f0a3 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 29 Nov 2011 16:17:34 +0100
Subject: [PATCH] Fix query to determine config channels in SSM

---
 .../common/db/datasource/xml/config_queries.xml|6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/xml/config_queries.xml b/java/code/src/com/redhat/rhn/common/db/datasource/xml/config_queries.xml
index 1484317..d11bd22 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/xml/config_queries.xml
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/xml/config_queries.xml
@@ -2096,8 +2096,8 @@ SELECT  CC.id, CC.name
  FROM rhnSet RS LEFT OUTER JOIN rhnServerConfigChannel SCC
  ON RS.element = SCC.server_id
 WHERE RS.label = 'system_list'
-AND SCC.config_channel_id = XXX.config_channel_id
-AND SCC.server_id = null) )
+AND SCC.config_channel_id is null
+AND SCC.server_id is null) )
 ORDER BY UPPER(CC.name)
   /query
   elaborator name=ssm_channel_elab /
@@ -2126,7 +2126,7 @@ SELECT  CC.id, CC.name
 FROM rhnSet RS LEFT OUTER JOIN rhnServerConfigChannel SCC
 ON RS.element = SCC.server_id
WHERE RS.label = 'system_list'
-   AND SCC.config_channel_id = XXX.config_channel_id
+   AND SCC.config_channel_id is null
AND SCC.server_id is null) )
 ORDER BY UPPER(CC.name)
   /query
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Remove markup from error messages

2011-11-15 Thread Johannes Renner
Hello,

Markup in error messages is now being escaped since this commit:

- a9da89b35581f4eedfb6ca7ddff4343b9ea21f15.

It looks like these strong fragments were forgotten when doing:

- e0c4ae8dd7093bbe6a12ab4462272fbab573e098 and
- 3a03e49904465ec5f16bde31c92e67c0b85ef1b9

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
From 8960b0ca3e8df8500f9129fd06493e3f3565cb86 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 15 Nov 2011 13:55:02 +0100
Subject: [PATCH] Remove markup from error message in all translation files

---
 .../frontend/strings/java/StringResource_bn_IN.xml |4 ++--
 .../frontend/strings/java/StringResource_de.xml|4 ++--
 .../frontend/strings/java/StringResource_en_US.xml |2 +-
 .../frontend/strings/java/StringResource_es.xml|4 ++--
 .../frontend/strings/java/StringResource_fr.xml|4 ++--
 .../frontend/strings/java/StringResource_gu.xml|4 ++--
 .../frontend/strings/java/StringResource_hi.xml|4 ++--
 .../frontend/strings/java/StringResource_it.xml|4 ++--
 .../frontend/strings/java/StringResource_ja.xml|4 ++--
 .../frontend/strings/java/StringResource_ko.xml|4 ++--
 .../frontend/strings/java/StringResource_pa.xml|4 ++--
 .../frontend/strings/java/StringResource_pt_BR.xml |4 ++--
 .../frontend/strings/java/StringResource_ru.xml|4 ++--
 .../frontend/strings/java/StringResource_ta.xml|4 ++--
 .../frontend/strings/java/StringResource_zh_CN.xml |4 ++--
 .../frontend/strings/java/StringResource_zh_TW.xml |4 ++--
 16 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_bn_IN.xml b/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_bn_IN.xml
index f268f29..d3f6ecd 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_bn_IN.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_bn_IN.xml
@@ -3130,13 +3130,13 @@ available for this kickstart profile to function properly.
   targetমূল প্রতিষ্ঠান {0} মুছে ফেলা যাবে না/target/trans-unit
 
   trans-unit id=userdisable.error.orgadmin
-sourceYou cannot deactivate lt;stronggt;anotherlt;/stronggt; organization
+sourceYou cannot deactivate another organization
 administrator. Please remove the 'Organization Administrator' role from this
 user before attempting to deactivate their account./source
 context-group name=ctx
 context context-type=sourcefile/rhn/users/DisableUser/context
 /context-group
-  targetআপনি lt;stronggt;অন্যান্যlt;/stronggt; প্রাতিষ্ঠানিক অ্যাডমিনিস্ট্রেটরের অ্যাকাউন্ট
+  targetআপনি অন্যান্য প্রাতিষ্ঠানিক অ্যাডমিনিস্ট্রেটরের অ্যাকাউন্ট
 নিষ্ক্রিয় করতে সক্ষম হবেন না। ব্যবহারকারীর অ্যাকাউন্ট নিষ্ক্রিয় করার পূর্বে অ্যাকাউন্ট থেকে
 'প্রাতিষ্ঠানিক অ্যাডমিনিস্ট্রেটরের' ভূমিকা সরিয়ে নিন।/target/trans-unit
 
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_de.xml b/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_de.xml
index e4b889c..ec0a3b7 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_de.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_de.xml
@@ -3155,13 +3155,13 @@ verfügbar machen, damit dieses Kickstart-Profil einwandfrei funktionieren kann.
   targetSie kamp;ouml;nnen die Basisorganisation {0} nicht lamp;ouml;schen/target/trans-unit
 
   trans-unit id=userdisable.error.orgadmin
-sourceYou cannot deactivate lt;stronggt;anotherlt;/stronggt; organization
+sourceYou cannot deactivate another organization
 administrator. Please remove the 'Organization Administrator' role from this
 user before attempting to deactivate their account./source
 context-group name=ctx
 context context-type=sourcefile/rhn/users/DisableUser/context
 /context-group
-  targetSie kamp;ouml;nnen keinen lt;stronggt;anderenlt;/stronggt; Organisations-
+  targetSie kamp;ouml;nnen keinen anderen Organisations-
 Administrator deaktivieren. Bitte entfernen Sie 'die Organisations-Administrator'-Funktion dieses
 Benutzers vor dem Versuch, den Account zu deaktivieren./target/trans-unit
 
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/java/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/java

[Spacewalk-devel] [PATCH] Pam setting on the user page not saving

2011-10-12 Thread Johannes Renner
Hello,

I just fixed a bug in Spacewalk about the pam setting on the user page not 
saving,
it can be reproduced like this:

1. Configure pam on the server (e.g. echo pam_auth_service = rhn-satellite  
/etc/rhn/rhn.conf)
2. Restart the tomcat
3. Log into spacewalk as an org_admin
4. In the page header click on username to edit the user's settings
5. Check on/off the use pam checkbox
6. Click on update

Result: The pam setting is not saved and the checkbox on the resulting page is
therefore shown in the same state as before.

(Note that it works to set the pam option for a user via Users - username)

Apply my attached patch for a fix, i.e. save the pam option using either
AdminUserEditAction.java or SelfUserEditAction.java.

Thanks,
Johannes

P.S.: I can also create a bugzilla entry if necessary, just tell me.

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
From 4abfbdcaea3ea3ddc5d9bb6aa1d478f2975d5723 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Wed, 12 Oct 2011 13:23:01 +0200
Subject: [PATCH] Fixed pam setting on user page not saving

---
 .../frontend/action/user/AdminUserEditAction.java  |   24 +--
 .../rhn/frontend/action/user/SelfEditAction.java   |3 ++
 .../frontend/action/user/UserEditActionHelper.java |   24 
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/AdminUserEditAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/AdminUserEditAction.java
index e34b726..fdcd29d 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/AdminUserEditAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/AdminUserEditAction.java
@@ -14,8 +14,6 @@
  */
 package com.redhat.rhn.frontend.action.user;
 
-import com.redhat.rhn.common.conf.Config;
-import com.redhat.rhn.common.conf.ConfigDefaults;
 import com.redhat.rhn.common.security.PermissionException;
 import com.redhat.rhn.domain.org.Org;
 import com.redhat.rhn.domain.role.Role;
@@ -85,26 +83,8 @@ public class AdminUserEditAction extends UserEditActionHelper {
 return returnFailure(mapping, request, errors, targetUser.getId());
 }
 
-/*
- * Update PAM Authentication attribute
- * If we're a satellite that is configured to use pam and the loggedIn user is an
- * org_admin (and therefore the checkbox was displayed), we need to inspect the
- * usepam field on the form and set the targetUser's pam auth attribute
- * accordingly. (we don't want to set this field if it wasn't displayed or if the
- * user doesn't have access to set this attribute)
- */
-String pamAuthService = Config.get().getString(ConfigDefaults.WEB_PAM_AUTH_SERVICE);
-if (pamAuthService != null 
-pamAuthService.trim().length()  0 
-loggedInUser.hasRole(RoleFactory.ORG_ADMIN)) {
-if (form.get(usepam) != null 
-((Boolean) form.get(usepam)).booleanValue()) {
-targetUser.setUsePamAuthentication(true);
-}
-else {
-targetUser.setUsePamAuthentication(false);
-}
-}
+// Update PAM Authentication attribute
+updatePamAttribute(loggedInUser, targetUser, form);
 
 //Create the user info updated success message
 createSuccessMessage(request, message.userInfoUpdated, null);
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/SelfEditAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/SelfEditAction.java
index d7f8dcc..cf3201b 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/SelfEditAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/SelfEditAction.java
@@ -48,6 +48,9 @@ public class SelfEditAction extends UserEditActionHelper {
 
 //If there are no errors, store the user and return the success mapping
 if (errors.isEmpty()) {
+// Update the PAM Authentication attribute
+updatePamAttribute(user, user, form);
+
 UserManager.storeUser(user);
 
 ActionMessages msgs = new ActionMessages();
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index c61fbc6..a9c1625 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -14,6 +14,9 @@
  */
 package com.redhat.rhn.frontend.action.user;
 
+import com.redhat.rhn.common.conf.Config;
+import com.redhat.rhn.common.conf.ConfigDefaults;
+import com.redhat.rhn.domain.role.RoleFactory;
 import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.struts.RhnAction;
 import

Re: [Spacewalk-devel] [PATCH] Pam setting on the user page not saving

2011-10-12 Thread Johannes Renner
On 10/12/2011 05:00 PM, Tomas Lestach wrote:
 Hello Johannes,
 
 yes, you're right.
 
 I just have one comment to your patch - I'd move updatePamAttribute() in 
 SelfEditAction.java file
 into the updateDetails method. If you're ok with the change, I'd commit it.
 Let me know.

Yes, I was just keeping things separated, but it seems as if there would be no
difference in the semantics. And you rather mean the class UserEditActionHelper,
right? You will then also need to add another parameter for the updateDetails()
method (the loggedInUser), but sure, commit it ;-)

Thanks,
Johannes

 Regards,
 -- 
 Tomas Lestach
 RHN Satellite Engineering, Red Hat
 
 
 On 10/12/2011 02:28 PM, Johannes Renner wrote:
 Hello,

 I just fixed a bug in Spacewalk about the pam setting on the user page not 
 saving,
 it can be reproduced like this:

 1. Configure pam on the server (e.g. echo pam_auth_service = 
 rhn-satellite  /etc/rhn/rhn.conf)
 2. Restart the tomcat
 3. Log into spacewalk as an org_admin
 4. In the page header click onusername  to edit the user's settings
 5. Check on/off the use pam checkbox
 6. Click on update

 Result: The pam setting is not saved and the checkbox on the resulting page 
 is
 therefore shown in the same state as before.

 (Note that it works to set the pam option for a user via Users -  
 username)

 Apply my attached patch for a fix, i.e. save the pam option using either
 AdminUserEditAction.java or SelfUserEditAction.java.

 Thanks,
 Johannes

 P.S.: I can also create a bugzilla entry if necessary, just tell me.




 ___
 Spacewalk-devel mailing list
 Spacewalk-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/spacewalk-devel
 
 ___
 Spacewalk-devel mailing list
 Spacewalk-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/spacewalk-devel

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] NPE when scheduling a package (install/remove/upgrade) action + remote command

2011-09-08 Thread Johannes Renner
Hey,

I am experiencing a bug, but I am working with Spacewalk 1.2 based code.
Unfortunately I don't have a recent spacewalk installation available to see
if it's happening there as well. It would be nice if somebody could try to
reproduce this issue, which would show me if I am just missing a recent commit
or if there is a bug. Here the instructions:

In the Web UI:

- Select a system via 'Systems' (click on one of the systems)
- Go to 'Software'
- Choose one of the options, e.g. 'Install New Packages'
- Choose one or more packages to be installed and submit
- Click on the button 'Add Remote Command to Package Install'
- Fill out the form (i.e. add script content) and submit

Result: There is an internal server error caused by a NullPointerException at
com.redhat.rhn.domain.action.script.ScriptAction.setScriptActionDetails(ScriptAction.java:38

If it can be reproduced, I will open a bug on bugzilla.redhat.com.

Here is my patch, but I'm not sure if this is the right way to go: After the
NPE is fixed within ScriptAction.java there appears to be a problem with the
redirect. Since HTTP GET method is used for the redirect, but the 'submitted'
parameter is still there, we get a 405. This can be fixed by removing the
'submitted' parameter before actually doing the redirect.

Thanks,
Johannes

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
From d9fa52242fdae2302d4d1ba06eb2c181df0ba6ed Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 8 Sep 2011 14:58:43 +0200
Subject: [PATCH] Fix NPE when scheduling a package action + remote command

---
 .../rhn/domain/action/script/ScriptAction.java |4 +++-
 .../action/rhnpackage/ScheduleRemoteCommand.java   |5 -
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/action/script/ScriptAction.java b/java/code/src/com/redhat/rhn/domain/action/script/ScriptAction.java
index 071a5eb..a0e453f 100644
--- a/java/code/src/com/redhat/rhn/domain/action/script/ScriptAction.java
+++ b/java/code/src/com/redhat/rhn/domain/action/script/ScriptAction.java
@@ -35,7 +35,9 @@ public class ScriptAction extends Action {
  * @param scriptActionDetailsIn The scriptActionDetails to set.
  */
 public void setScriptActionDetails(ScriptActionDetails scriptActionDetailsIn) {
-scriptActionDetailsIn.setParentAction(this);
+if (scriptActionDetailsIn != null) {
+scriptActionDetailsIn.setParentAction(this);
+}
 scriptActionDetails = scriptActionDetailsIn;
 }
 
diff --git a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/ScheduleRemoteCommand.java b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/ScheduleRemoteCommand.java
index d3a9ae8..40b2481 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/ScheduleRemoteCommand.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/rhnpackage/ScheduleRemoteCommand.java
@@ -40,6 +40,7 @@ import org.apache.struts.action.DynaActionForm;
 
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -92,9 +93,11 @@ public class ScheduleRemoteCommand extends RhnAction {
 strutsDelegate.saveMessages(request, msgs);
 
 String mode = (String) f.get(mode);
+Map params = new HashMap(request.getParameterMap());
+params.remove(RhnAction.SUBMITTED);
 forward = strutsDelegate.forwardParams(
 mapping.findForward(getForward(mode)),
-request.getParameterMap());
+params);
 }
 
 return forward;
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Error message when successfully toggling user roles

2011-09-06 Thread Johannes Renner
Hey,

Here is a (very) small thing I stumbled upon: There is an error message
sent to the client even in case of success while toggling administrator
roles of single users. Please see the attached patch for a fix to send
a success message instead. There is further no bugzilla item about this,
since it was probably never an issue..

(The URI of the respective page is /rhn/admin/multiorg/Users.do)

Greetings,
Johannes

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
From 9138ab493776b334005c8ab63f635284b913da6e Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 6 Sep 2011 10:03:55 +0200
Subject: [PATCH] Do not send error message in case of success

---
 .../frontend/action/multiorg/SatAdminAction.java   |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/multiorg/SatAdminAction.java b/java/code/src/com/redhat/rhn/frontend/action/multiorg/SatAdminAction.java
index 479a7ad..3fe048c 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/multiorg/SatAdminAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/multiorg/SatAdminAction.java
@@ -76,12 +76,12 @@ public class SatAdminAction extends RhnAction {
 // check role and toggle
 if (u.hasRole(RoleFactory.SAT_ADMIN)) {
 u.removeRole(RoleFactory.SAT_ADMIN);
-createErrorMessage(request, user.satadmin.remove,
+createSuccessMessage(request, user.satadmin.remove,
 u.getLogin());
 }
 else {
 u.addRole(RoleFactory.SAT_ADMIN);
-createErrorMessage(request, user.satadmin.add,
+createSuccessMessage(request, user.satadmin.add,
 u.getLogin());
 }
 
-- 
1.7.3.4


___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Audit Logging

2011-08-26 Thread Johannes Renner
On 08/26/2011 11:02 AM, Miroslav Suchý wrote:
 On 08/25/2011 06:55 PM, Johannes Renner wrote:
 Well, I can count three, while only one (!) of them is public. But we
 can still rename the other ones, no problem.
 
 I count:
   private static Logger log = Logger.getLogger(AuditLog.LOGGER_NAME);
 as well.

While this is not a method and also not a function, however ...

 - How can you distinguish between interesting and uninteresting requests?
 - There is some interesting requests that are not POST requests, e.g. 
 logouts.
 - There is some POST requests that are not interesting, e.g user selects all
   entries of a list.
 - Log events cannot be categorized for a later filtering. At a single entry
   point it is very hard to see what really has happened.
 
 I thought that there will be configuration file, where you state what
 and how will be logged. All based on URI similary to struts config file.
 E.g.
 
 /rhn/LoginSubmit.do {
key = LOGIN
value = user=${POST.username};pass=${POST.password}
 }
 
 /rhn/admin/config/GeneralConfig.do {
key = CONF
value = email=${POST.email};.
 }
 
 etc. you probably got the idea now.
 And those url not specified will not be logged.

I got the idea and I was even researching such an approach already. For the
webapp I started with a ServletFilter (see my attached patch as an example)
that simply logs all POST requests to the backend using my helper classes
from the patches I already sent. The main thing that's missing would be the
integration of a configuration file like the above.

I will now continue to investigate in this approach since I agree with you
that it will be much easier to maintain than having log statements all over
the code. However there is also some drawbacks:

- Performance might be worse, since _every_ request this filter is registered
  for (e.g. all *.do) will be checked if it needs to be logged or not
- Sometimes the same URL is used for different actions, e.g. creating and
  updating an object, so classification of log events might be difficult or
  even not possible
- Sometimes you only want to log the request in case a certain parameter is
  there, so there would need to be a something like a list of parameters
  required for logging for each URL in the config
- does it make sense to have a whitelist of interesting parameters for each
  URL or rather take everything and maintain a global blacklist?

 - When using an external entry point (like mod_security), you can't actually
   see from the logs which user was involved since it is not possible to map
   between uid, sid, ... and real world 'objects'.
 
 I said something liek mod_security. I can imagine build something upon
 existing project, but even something new written from scratch just for
 Spacewalk.
 And translating sid to user is not so big problem. You can have config
 file where you specify how you translate sid to user. Ie.
 [translate]
 user = select login from web_contact join pxtsession on
 web_user_id=web_contact.id where pxt.id = :sid
 
 and in logging config have:
 /rhn/admin/config/GeneralConfig.do {
key = CONF
translate[user] = sid
value = logged=${user};email=${POST.email};.
 }
 
 This way it can be even Spacewalk independent and you can use it on
 different project where they have different tables.

Yes, but I actually think it would make sense to do this specifically within
spacewalk-java, because there is already code to determine all the stuff from
the database. To me it would make sense to reuse this code, so we don't need
to rewrite all those queries?

 I agree with you completely on the fact that getting the big picture is hard,
 but generic logging of request data does somehow not satisfy our needs :-/
 
 So there is place to write one :)
 Just think that after some years customer will ask you and which
 events/action/url are logged? Can you give me the list.
 And you will have hard time to provide such list.

Yes, and such a configuration could even be modified by a customer itself.

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
diff --git a/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java b/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java
new file mode 100644
index 000..df3c728
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java
@@ -0,0 +1,70 @@
+/**
+ * Copyright (c) 2011 Novell
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use

Re: [Spacewalk-devel] Audit Logging

2011-08-25 Thread Johannes Renner
On 08/25/2011 05:43 PM, Miroslav Suchý wrote:
 On 08/25/2011 02:30 PM, Johannes Renner wrote:
 So (1.) there are some helper classes that I put into a new separate
 package, see 01-new-classes.patch:
 
 AuditLog.java has 4 (!) functions log() and all of them has different
 semantics - if I ommit that they are utilized to logging. Please
 consider different names.

Well, I can count three, while only one (!) of them is public. But we
can still rename the other ones, no problem.


 Therefore there is (2.) calls to the logger from within the existing
 code, see 03-rhnaction-example.patch:
 
 Hmm, what I dislike is the need to change the code. Which will be prone
 to coder error and you may end up in situtation where somebody commit
 code, which are worth auditing, but the audit call will not be there.
 
 If I would be designing such functionality I would prefer something like
 mod_security. So you will have one entry point and you will not care if
 the action is actually handled by cobbler, backend, frontend or some aliens.
 This would even allow you to have configuration file where you will
 define what and how will be audited.
 In your design it will be hard to say which actions are audited.
 Especially in 2 years from now, when other will add/remove/change your
 initial audit calls.

I completely agree and also dislike the need to change the code, but:

If we wanted to log only HTTP requests and responses we could very well
use something like mod_security, or alternatively use a ServletFilter or
put the implementation into RhnRequestProcessor. Such an implementation
would be much easier and there would be a single entry point, but there
is several disadvantages as well, e.g.:

- How can you distinguish between interesting and uninteresting requests?
- There is some interesting requests that are not POST requests, e.g. logouts.
- There is some POST requests that are not interesting, e.g user selects all
  entries of a list.
- Log events cannot be categorized for a later filtering. At a single entry
  point it is very hard to see what really has happened.
- When using an external entry point (like mod_security), you can't actually
  see from the logs which user was involved since it is not possible to map
  between uid, sid, ... and real world 'objects'.

So, it depends a bit on how the requirements are, but to me it looks like
all generic solutions will unfortunately not really meet our requirements.

 - While most of the struts actions are RhnAction or RhnLookupDispatchAction,
   there is still some that are not derived from these classes. Therefore it's
   not enough to put a shortcut method to the logger in the(se) 
 superclass(es).
 
 Exactly, if you go from bottom (base classes) you can be never sure if
 log all childs and getting the big picture will be very hard. And you
 could not be really sure till you test it.

It's supposed to be done in the superclass, this is just a shortcut for
making it easier to put the calls in the derived classes where 'interesting'
things (mostly calls to whatever *Manager class) are actually happening.

I agree with you completely on the fact that getting the big picture is hard,
but generic logging of request data does somehow not satisfy our needs :-/

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


Re: [Spacewalk-devel] [PATCH] Synchronization to Software Profiles

2011-07-29 Thread Johannes Renner
On 07/29/2011 04:33 PM, Cliff Perry wrote:
 On 07/29/2011 09:55 AM, Johannes Renner wrote:
 Hello,

 I fixed a subtle bug in the spacewalk web UI that caused the scheduling
 of a system's synchronization to a software profile to not work anymore.
 The patch in the attachment should fix the problem.
 
 Good catch. Do you know if there is a bugzilla (Satellite or Spacewalk or 
 Manager) which reported
 this issue?
 
 Thanks,
 Cliff

I do not know about a bugzilla item on the redhat bugzilla and I couldn't find
any during a quick search. There is one on the bugzilla at novell, but this is
unfortunately not open to the public. Shall I create one on redhat.bugzilla.com?

Greetings,
Johannes


 For reproducing the problem first go to Systems and select a system. Then
 go to Software -  Packages -  Profiles, create a profile, compare to it
 and try to sync.

 Thank you,
 Johannes

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Refactor listing of channels: RedHat.do - Vendor.do

2011-07-05 Thread Johannes Renner
Hello,

for making it more generic, we refactored the web UI vendor channel listing
from 'RedHat.do' to 'Vendor.do'. Unfortunately, doing this in a consistent
way includes changes in quite an amount of files. If this refactoring is
still acceptable for upstream, you might want to apply my attached patch.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
From 506bbd5b7df16f0bdf04de8b31b3b4fc4f80b5c7 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 5 Jul 2011 11:47:55 +0200
Subject: [PATCH] Refactor RedHat.do to Vendor.do

---
 .../common/db/datasource/xml/Channel_queries.xml   |2 +-
 .../action/channel/RedHatChannelTreeAction.java|   35 --
 .../action/channel/VendorChannelTreeAction.java|   35 ++
 .../frontend/strings/jsp/StringResource_bn_IN.xml  |2 +-
 .../rhn/frontend/strings/jsp/StringResource_de.xml |2 +-
 .../frontend/strings/jsp/StringResource_en_US.xml  |2 +-
 .../rhn/frontend/strings/jsp/StringResource_es.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_fr.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_gu.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_hi.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_it.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_ja.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_ko.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_pa.xml |2 +-
 .../frontend/strings/jsp/StringResource_pt_BR.xml  |2 +-
 .../rhn/frontend/strings/jsp/StringResource_ru.xml |2 +-
 .../rhn/frontend/strings/jsp/StringResource_ta.xml |2 +-
 .../frontend/strings/jsp/StringResource_zh_CN.xml  |2 +-
 .../frontend/strings/jsp/StringResource_zh_TW.xml  |2 +-
 .../frontend/strings/nav/StringResource_en_US.xml  |2 +-
 .../frontend/xmlrpc/channel/ChannelHandler.java|2 +-
 .../redhat/rhn/manager/channel/ChannelManager.java |8 ++--
 .../manager/channel/test/ChannelManagerTest.java   |4 +-
 java/code/webapp/WEB-INF/nav/channel_tabs.xml  |4 +-
 .../webapp/WEB-INF/nav/sitenav-authenticated.xml   |2 +-
 java/code/webapp/WEB-INF/pages/channel/redhat.jsp  |   38 
 java/code/webapp/WEB-INF/pages/channel/vendor.jsp  |   38 
 java/code/webapp/WEB-INF/struts-config.xml |8 ++--
 web/html/nav/sitenav-authenticated.xml |2 +-
 29 files changed, 106 insertions(+), 106 deletions(-)
 delete mode 100644 java/code/src/com/redhat/rhn/frontend/action/channel/RedHatChannelTreeAction.java
 create mode 100644 java/code/src/com/redhat/rhn/frontend/action/channel/VendorChannelTreeAction.java
 delete mode 100644 java/code/webapp/WEB-INF/pages/channel/redhat.jsp
 create mode 100644 java/code/webapp/WEB-INF/pages/channel/vendor.jsp

diff --git a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Channel_queries.xml b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Channel_queries.xml
index 655639f..f2bdac4 100644
--- a/java/code/src/com/redhat/rhn/common/db/datasource/xml/Channel_queries.xml
+++ b/java/code/src/com/redhat/rhn/common/db/datasource/xml/Channel_queries.xml
@@ -443,7 +443,7 @@ SELECT CFO.id, CFO.org_id, CFO.name, CFO.label, CFO.current_members, CFO.max_mem
 
 
 
-mode name=redhat_channel_tree class=com.redhat.rhn.frontend.dto.ChannelTreeNode
+mode name=vendor_channel_tree class=com.redhat.rhn.frontend.dto.ChannelTreeNode
 	query params=user_id
 	select Distinct C.id,
 		   C.name,
diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/RedHatChannelTreeAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/RedHatChannelTreeAction.java
deleted file mode 100644
index f5b61d9..000
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/RedHatChannelTreeAction.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/**
- * Copyright (c) 2009--2010 Red Hat, Inc.
- *
- * This software is licensed to you under the GNU General Public License,
- * version 2 (GPLv2). There is NO WARRANTY for this software, express or
- * implied, including the implied warranties of MERCHANTABILITY or FITNESS
- * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
- * along with this software; if not, see
- * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
- *
- * Red Hat trademarks are not licensed under GPLv2. No permission is
- * granted to use or replicate Red Hat trademarks that are incorporated
- * in this software or its documentation.
- */
-package com.redhat.rhn.frontend.action.channel;
-
-import com.redhat.rhn.common.db.datasource.DataResult;
-import com.redhat.rhn.domain.user.User;
-import com.redhat.rhn.frontend.listview.ListControl;
-import com.redhat.rhn.frontend.struts.RequestContext;
-import com.redhat.rhn.manager.channel.ChannelManager;
-
-/**
- * ChannelTreeRelevantSetupAction
- * @version $Rev

Re: [Spacewalk-devel] [PATCH] Refactor listing of channels: RedHat.do - Vendor.do

2011-07-05 Thread Johannes Renner
On 07/05/2011 12:23 PM, Johannes Renner wrote:
 Hello,
 
 for making it more generic, we refactored the web UI vendor channel listing
 from 'RedHat.do' to 'Vendor.do'. Unfortunately, doing this in a consistent
 way includes changes in quite an amount of files. If this refactoring is
 still acceptable for upstream, you might want to apply my attached patch.

Another thing we'd like to propose is renaming the API method 
'listRedHatChannels()'
to the more generic 'listVendorChannels()'. This can of course be done while 
keeping
the old method as an alias to the refactored one, but marking it as deprecated.

Please see the attached patch.

Thank you,
Johannes

-- 
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
From 26ec3434e73090155d267b0f6deee4f1ec402ff5 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 5 Jul 2011 12:48:33 +0200
Subject: [PATCH] Refactor and deprecate API method to listVendorChannels

---
 .../frontend/xmlrpc/channel/ChannelHandler.java|   26 +--
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/ChannelHandler.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/ChannelHandler.java
index a1135b0..a584ee3 100644
--- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/ChannelHandler.java
+++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/ChannelHandler.java
@@ -121,12 +121,12 @@ public class ChannelHandler extends BaseHandler {
 }
 
 /**
- * Lists all Red Hat software channels that the user's organization is entitled to.
+ * Lists all of the vendor's software channels that the user's organization is entitled to.
  * @param sessionKey session containing User information.
  * @return Returns array of channels with info such as channel_label, channel_name,
  * channel_parent_label, packages and systems.
  *
- * @xmlrpc.doc List all Red Hat software channels that the user's organization is
+ * @xmlrpc.doc List all of the vendor's software channels that the user's organization is
  * entitled to.
  * @xmlrpc.param #session_key()
  * @xmlrpc.returntype
@@ -134,7 +134,7 @@ public class ChannelHandler extends BaseHandler {
  * $ChannelTreeNodeSerializer
  * #array_end()
  */
-public Object[] listRedHatChannels(String sessionKey) {
+public Object[] listVendorChannels(String sessionKey) {
 User user = ChannelHandler.getLoggedInUser(sessionKey);
 DataResultChannelTreeNode dr = ChannelManager.vendorChannelTree(user, null);
 dr.elaborate();
@@ -142,6 +142,26 @@ public class ChannelHandler extends BaseHandler {
 }
 
 /**
+ * Lists all Red Hat software channels that the user's organization is entitled to.
+ * @param sessionKey session containing User information.
+ * @return Returns array of channels with info such as channel_label, channel_name,
+ * channel_parent_label, packages and systems.
+ * @deprecated being replaced by listVendorChannels(String sessionKey)
+ *
+ * @xmlrpc.doc List all Red Hat software channels that the user's organization is
+ * entitled to.
+ * @xmlrpc.param #session_key()
+ * @xmlrpc.returntype
+ * #array()
+ * $ChannelTreeNodeSerializer
+ * #array_end()
+ */
+@Deprecated
+public Object[] listRedHatChannels(String sessionKey) {
+return listVendorChannels(sessionKey);
+}
+
+/**
  * Lists the most popular software channels based on the popularity
  * count given.
  * @param sessionKey session containing User information.
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] Password Strength Verification

2011-05-10 Thread Johannes Renner
Hello,

I was recently investigating in hardening security in the spacewalk web-app by
introducing password strength verification for new passwords, which means 
forcing
the users to choose passwords with a certain strength. It currently seems to me 
as
if there are two options that I listed below with my personal pros(+) and 
cons(-).
So, which implementation would you prefer and why?

1. Write a custom password strength verificator in Java (like in e.g. ESAPI 
[1]):

+ not hard to implement (at least when omitting dictionary lookups)
+ requirements can be made configurable, e.g. password min/max length
- no dictionary lookups

2. Write a wrapper around the 'cracklib-check' binary:

+ backend is a well known and tested library (cracklib)
+ comes with an integrated dictionary lookup
- introduces a dependency on a 3rd party binary
- strength requirements seem to be not configurable

Greetings,
Johannes

[1]
http://owasp-esapi-java.googlecode.com/svn/trunk_doc/latest/org/owasp/esapi/Authenticator.html#verifyPasswordStrength%28java.lang.String,%20java.lang.String,%20org.owasp.esapi.User%29

-- 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 
16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Do not show success message when passwords don't match

2011-03-28 Thread Johannes Renner
Hello,

I found it to be confusing, that if you go to the account preferences
page of a user (/rhn/account/UserDetails.do) and change the user's
confirm password only (e.g. remove one of the dots), the message when
you hit the 'Update' button states:

'User information updated'

I'm not 100% sure about the expected behavior there, but I think if
both passwords don't match we actually shouldn't display a success
message (even if one of the passwords is the placeholder). The attached
patch refines the class UserEditActionHelper and adds the password
mismatch error as soon as both passwords don't match.

Greetings,
Johannes

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
From 5c07c2b9824ff125a0d5f66426b3fe996d94a213 Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Mon, 28 Mar 2011 13:45:25 +0200
Subject: [PATCH] Do not show success message when passwords don't match

---
 .../frontend/action/user/UserEditActionHelper.java |   18 ++
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index 68f6ce0..c455642 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -40,16 +40,18 @@ public abstract class UserEditActionHelper extends RhnAction {
 //get validation errors
 ActionErrors errors = RhnValidationHelper.validateDynaActionForm(this, form);
 
-//Make sure password and passwordConfirm are equal
+//Add an error in case of password mismatch
+String pw = (String)form.get(UserActionHelper.DESIRED_PASS);
+String conf = (String)form.get(UserActionHelper.DESIRED_PASS_CONFIRM);
+if (!pw.equals(conf)) {
+errors.add(ActionMessages.GLOBAL_MESSAGE,
+new ActionMessage(error.password_mismatch));
+}
+
+//Make sure password is not the placeholder
 if (!UserActionHelper.PLACEHOLDER_PASSWORD.equals(
 form.get(UserActionHelper.DESIRED_PASS))) {
-String pw = (String)form.get(UserActionHelper.DESIRED_PASS);
-String conf = (String)form.get(UserActionHelper.DESIRED_PASS_CONFIRM);
-if (!pw.equals(conf)) {
-errors.add(ActionMessages.GLOBAL_MESSAGE,
-new ActionMessage(error.password_mismatch));
-}
-else if (errors.isEmpty()) {
+if (errors.isEmpty()) {
 //Set the password only if there are no errors at all
 targetUser.setPassword(pw);
 }
-- 
1.7.3.4

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] Hide the 'delete note' link on creation of system notes

2011-03-01 Thread Johannes Renner
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hello,

Attached is a small patch for the jsp page that is used for editing
system notes. It hides the delete note link when a system note is
currently being created. This is done by including the toolbar within
the c:choose element. Clicking this link during the creation of a
new note will otherwise lead to internal server error (NPE), because
(of course) the nid is still null.

Regards,
Johannes Renner

- -- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNbMnuAAoJEGYafiwbDR5pQVAH/i2Wrx2jaO3ZvQBTWKSqxVkX
rlCIJrAXEXrYyBF/IkPgBCZ9BZ26A4sQo494Sb8stTAZZ+WdLsERgVqOoa3bSxBw
5+qOnogY581FPuEn/3OnP0hQ+JV8tktUb33L4qw7pJCtz0TTEEs3uwZt8XZ5/a/I
nH66i/Yil4DebmytXDZrvcwFmWCdCeIcDSxpERXHzwT64OcAfCIM/Aes4Lwq/J8l
KgXyqpV3y2zgA+ID1m2gsSQ7waKWVfLDdZt90nB0Fv1GpScmFBHB6d4yCkY31ER/
xJ9vDuID3c/Fq8St1ffh3sReCWTZzIeFX5E9FNBlvJfXAJCcJPzZHa31avTsDu4=
=MqUC
-END PGP SIGNATURE-
From ea324e8b7b6907f24d010b7b4439bc1e00e8f8bf Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Thu, 17 Feb 2011 11:20:52 +0100
Subject: [PATCH] do not show delete link on creation of notes (bnc#672090)

---
 .../webapp/WEB-INF/pages/systems/sdc/note_edit.jsp |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/java/code/webapp/WEB-INF/pages/systems/sdc/note_edit.jsp b/java/code/webapp/WEB-INF/pages/systems/sdc/note_edit.jsp
index 408780d..87d60f4 100644
--- a/java/code/webapp/WEB-INF/pages/systems/sdc/note_edit.jsp
+++ b/java/code/webapp/WEB-INF/pages/systems/sdc/note_edit.jsp
@@ -9,18 +9,19 @@
 
 %@ include file=/WEB-INF/pages/common/fragments/systems/system-header.jspf %
 
-rhn:toolbar base=h2 img=/img/rhn-icon-note.gif
-   deletionUrl=/rhn/systems/details/DeleteNote.do?sid=${system.id}nid=${n.id}
-   deletionType=note
-  
-  bean:message key=sdc.details.notes.header/
-/rhn:toolbar
-
 c:choose
   c:when test=${empty param.nid}
+rhn:toolbar base=h2 img=/img/rhn-icon-note.gif
+  bean:message key=sdc.details.notes.header/
+/rhn:toolbar
 c:set var=urlParam scope=request value=sid=${system.id}/
   /c:when
   c:otherwise
+rhn:toolbar base=h2 img=/img/rhn-icon-note.gif
+deletionUrl=/rhn/systems/details/DeleteNote.do?sid=${system.id}nid=${n.id}
+deletionType=note
+  bean:message key=sdc.details.notes.header/
+/rhn:toolbar
 c:set var=urlParam scope=request value=sid=${system.id}nid=${n.id}/
   /c:otherwise
 /c:choose
-- 
1.7.1



hide-delete-link-on-creation-of-notes.patch.sig
Description: PGP signature
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Hide the 'delete note' link on creation of system notes

2011-03-01 Thread Johannes Renner
On 03/01/2011 02:29 PM, Jan Pazdziora wrote:
 By the way: will you be submitting the patch for the Has Signed
 Metadata feature as well?

Yes, this one is also going to be submitted, but unfortunately I can't
tell you exactly when. The problem is that this is currently not a
single patch, but rather a set of patches that will need to be collected
and aggregated first. So, as soon as we have done that and tested it
against the master branch it will also be submitted here.

Greetings,
Johannes

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


[Spacewalk-devel] [PATCH] Password with less than minlength characters accepted

2011-01-27 Thread Johannes Renner
Hello,

There seems to be a bug in the Spacewalk Java code that allows a user
to set whatever password regardless of any errors (e.g. length 
minlength), as long as the desired and confirm password are equal. It
is even possible to set a user's password to the empty string, which
results in not being able to login anymore after sign out! Attached is
a patch that fixes the problem.

Greetings,
Johannes Renner

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)


From 57bac09f19795e5fc4c21514da4ac20bef9cc79d Mon Sep 17 00:00:00 2001
From: Johannes Renner jren...@suse.de
Date: Tue, 18 Jan 2011 15:58:23 +0100
Subject: [PATCH] Password with less than minlength characters accepted

---
 .../frontend/action/user/UserEditActionHelper.java |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index 79529b4..68f6ce0 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -45,12 +45,13 @@ public abstract class UserEditActionHelper extends RhnAction {
 form.get(UserActionHelper.DESIRED_PASS))) {
 String pw = (String)form.get(UserActionHelper.DESIRED_PASS);
 String conf = (String)form.get(UserActionHelper.DESIRED_PASS_CONFIRM);
-if (pw.equals(conf)) {
-targetUser.setPassword(pw);
-}
-else {
+if (!pw.equals(conf)) {
 errors.add(ActionMessages.GLOBAL_MESSAGE,
-   new ActionMessage(error.password_mismatch));
+new ActionMessage(error.password_mismatch));
+}
+else if (errors.isEmpty()) {
+//Set the password only if there are no errors at all
+targetUser.setPassword(pw);
 }
 }
 
-- 
1.7.1



___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel