Bug 531122

Looks like everytime activation key associationss are modified for a ks profile (whether add or remove), it syncs to the cobbler profile.  Somewhere along the way, it loses the previous values, it may be CobblerObject.getRedHatManagementKey already doesn't return the right set from the data map initialization, or when modifying it... 

Relevant code snippets:

src/com/redhat/rhn/frontend/action/kickstart/ActivationKeysSubmitAction.java:
...
  60     protected void operateOnRemovedElements(List elements,

  61                                             HttpServletRequest request) {

  62         RequestContext ctx = new RequestContext(request);

  63 

  64         KickstartActivationKeysCommand cmd =

  65             new KickstartActivationKeysCommand(

  66                     ctx.getRequiredParam(RequestContext.KICKSTART_ID),

  67                     ctx.getCurrentUser());

  68 
...   

  82         cmd.removeTokensByIds(ids);

  83         cmd.store();

  84 

  85         return;

  86     }
...
  92     protected void operateOnAddedElements(List elements, HttpServletRequest request) {

  93         RequestContext ctx = new RequestContext(request);

  94 

  95         KickstartActivationKeysCommand cmd =

  96             new KickstartActivationKeysCommand(

  97                     ctx.getRequiredParam(RequestContext.KICKSTART_ID), 

  98                     ctx.getCurrentUser());
...
 113         cmd.addTokensByIds(ids);

 114         cmd.store();

 115 

 116         return;

 117     }
...

src/com/redhat/rhn/manager/kickstart/KickstartActivationKeysCommand.java:
...
  49     public void removeTokensByIds(ArrayList<Long> ids) {

  50         Set<String> keysToRemove = new HashSet<String>();

  51 

  52         for (Long id : ids) {

  53             Set<Token> tokenSetCopy = new HashSet<Token>();

  54             tokenSetCopy.addAll(this.getKickstartData().getDefaultRegTokens());

  55             for (Token token : tokenSetCopy) {

  56                 if (token.getId() == id) {

  57                     this.getKickstartData().getDefaultRegTokens().remove(token);

  58                     keysToRemove.add(ActivationKeyFactory.lookupByToken(token).getKey());

  59                 }

  60             }

  61         }

  62 

  63         Profile prof = Profile.lookupById(

  64                 CobblerXMLRPCHelper.getConnection(this.getUser()),

  65                 this.getKickstartData().getCobblerId());

  66         if (prof != null) {

  67             prof.syncRedHatManagementKeys(keysToRemove, Collections.EMPTY_SET);

  68         }

  69         prof.save();

  70     }
...
  76     public void addTokensByIds(ArrayList<Long> ids) {

  77         Set<String> keysToAdd = new HashSet<String>();

  78 

  79         for (Long id : ids) {

  80             Token token = TokenFactory.lookupById(id);

  81             this.getKickstartData().addDefaultRegToken(token);

  82             keysToAdd.add(ActivationKeyFactory.lookupByToken(token).getKey());

  83         }

  84         Profile prof = Profile.lookupById(

  85                 CobblerXMLRPCHelper.getConnection(this.getUser()),

  86                 this.getKickstartData().getCobblerId());

  87         if (prof != null) {

  88             prof.syncRedHatManagementKeys(Collections.EMPTY_SET, keysToAdd);

  89         }

  90         prof.save();

  91     }

  92 
...

src/org/cobbler/Profile.java:
...
  32 public class Profile extends CobblerObject {
...
 396       public void syncRedHatManagementKeys(Collection<String> keysToRemove,

 397                                                   Collection<String> keysToAdd) {

 398           super.syncRedHatManagementKeys(keysToRemove, keysToAdd);

 399           for (SystemRecord record :

 400                       SystemRecord.listByAssociatedProfile(client, this.getName())) {

 401               record.syncRedHatManagementKeys(keysToRemove, keysToAdd);

 402               record.save();

 403           }

 404       }
...

src/org/cobbler/CobblerObject.java:
...
  56     private static final String REDHAT_KEY = "redhat_management_key";
...
 473     public String getRedHatManagementKey() {

 474         return (String) dataMap.get(REDHAT_KEY);

 475     }
...
 481     public Set<String> getRedHatManagementKeySet() {

 482         String keys = StringUtils.defaultString(getRedHatManagementKey());

 483         String[] sets = (keys).split(",");

 484         Set set = new HashSet();

 485         set.addAll(Arrays.asList(sets));

 486         return set;

 487     }
...
 494     public void syncRedHatManagementKeys(Collection<String> keysToRemove,

 495                                             Collection<String> keysToAdd) {

 496         Set keySet = getRedHatManagementKeySet();

 497         keySet.removeAll(keysToRemove);

 498         keySet.addAll(keysToAdd);

 499         setRedHatManagementKey(keySet);

 500     }
...

For now, workaround is to manual change the cobbler-generated pxelinux file for the system and pointing it at the first (base?) profile.

4a13f480c34ea364fb285c80baf5bfcc832e6df1

fixed in spacewalk master.

satellite.git: 68422b1d6e83442c60ef5dd511795fe5ee0d9ec4

# REOPEN

The issue is not fixed.

Reproducer was successful on initial packages of sat530

And proceeding the scenario in comment#0 on updated packages brings to following:
The cobbler update of new adding act. key to the kickstart profile TOTALLY overwrites *all* the previous act keys (including <<inherit>>) so the "--activationkey=<new_key>" looks like this was, but should be: "--activationkey=<<inherit>>,<old_key>,<new_key>"

@Justin: feel free contact me for the env. to reproduce the error. thanks.

Hey Garik,

It looks like the reproducer machine is running spacewalk-java-0.5.44-67.el4sat  but i don't think those changes landed until spacewalk-java-0.5.44-70.el4sat.

Can you retest with the newest spacewalk-java?

Thanks!

-Justin

Finally figured out the error you were referring to and it is related to creating the system record from cobbler directly.  If you do it from the UI it works fine.   The reason was that we weren't attempting to add all existing keys when updating a kickstart profile, only the new ones.  Made a change to add all the keys even if they are already there (dups are ignored).

Spacewalk commit: 
624f3267af6ab45f84753d1c4e5461ea19b0fc6d

Satellite commit:702746070eb6d8ba3824f475d4cebf7e59f1cb5d

# VERIFIED

Checking the scenario in comment#0 also the case with creating Cobbler System Record passed okey.

Add/remove of the activation keys in the cobbler profiles are updating properly - nothing is getting lost now.

packages fixing the issue are:
---
python-cheetah-2.0.1-1
cobbler-1.6.6-5
spacewalk-web-0.5.23-32
spacewalk-java-0.5.44-73
spacewalk-backend-0.5.28-40.x.2
---

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2010-0369.html