Bug 495506 - package details, not showing Change log and File List
package details, not showing Change log and File List
Status: CLOSED CURRENTRELEASE
Product: Red Hat Satellite 5
Classification: Red Hat
Component: WebUI (Show other bugs)
530
All Linux
low Severity medium
: ---
: ---
Assigned To: Jay Dobies
wes hayutin
https://grandprix.rhndev.redhat.com/r...
: Regression
Depends On:
Blocks: 456985
  Show dependency treegraph
 
Reported: 2009-04-13 11:04 EDT by wes hayutin
Modified: 2009-09-10 16:35 EDT (History)
4 users (show)

See Also:
Fixed In Version: sat530
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-10 16:35:31 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description wes hayutin 2009-04-13 11:04:12 EDT
Description of problem:

4/9 build rhel 5 x86

Navigate to a package details page.
Notice you can not see the sub items for 
Change Log
and
File List

Click on Dependencies, now the two links
appear.  I *think* the two items should 
always be visible.

recreated the issues while checking a few other packages
like squid..

thanks
Comment 1 Clifford Perry 2009-04-14 10:57:41 EDT
Page was converted from perl to java. Assume we forgot to update links/acls on the 
/rhn/software/packages/Details.do

page vs old 
network/software/packages/details.pxt

Should be easy to fix the links to display all 3 additional tabs and not just the # Dependencies sub tab. 

Cliff.
Comment 2 Shannon Hughes 2009-04-14 11:53:25 EDT
i looked at this before cperry assigned to jdobies. acls look ok in the navs on both perl/java stack. what is odd is after a restart or compile and restart the page displays ok. the package map that is returned by the java code does not have the archtype key in it for the refresh. the problem is either in the package acl handler or the code it calls.
Comment 3 Jay Dobies 2009-04-21 11:00:38 EDT
commit	a16f85fd6a4273cd1a4939660b9a65fe051ab455
tree	9475b6569e1a3a899095e8cdf06d65d5964415a2

ArchType.java

ArchType needed a defined equals/hashCode method. Otherwise, the capability lookup by type was subject to object references and was non-deterministic.
Comment 4 Jay Dobies 2009-04-21 11:01:25 EDT
Vader

commit	b58629a60a0d2432d1a72b5dc9967008a6242f5f
tree	524286c26d57265391a10f00909f2070d2983847
Comment 5 wes hayutin 2009-04-23 14:23:48 EDT
not in the 422.2 build moving back to modified
Comment 6 Jay Dobies 2009-04-27 11:34:44 EDT
Mass move to ON_QA for build on Apr 24.
Comment 7 wes hayutin 2009-04-27 16:39:23 EDT
same issue, may not have made it into the 4/24.1 build
Comment 8 wes hayutin 2009-05-11 12:55:20 EDT
fails.. not in 5/7.1 build
Comment 9 Jay Dobies 2009-05-12 15:21:51 EDT
Master
commit	3363f86e4acadcbbaedadc4e5f8076fb29aa27de
tree	19d1435af088a67ded5c189d488e01f9c44352ef

Vader
commit	39143ce1015882ff163a16f1a62fffa8c675a09c
tree	f14f924ceef40ba99d38555136517ccc74da6c80

java/code/src/com/redhat/rhn/common/security/acl/PackageAclHandler.java
java/code/src/com/redhat/rhn/domain/common/ArchType.java

This only seems to occur on the first run of a new ISO installation. I temporarily increased the logging on why the ACL fails so we capture as much as possible on the rare occasion it fails. Will remove the logging (it's annoyingly chatty) after the next QA build.
Comment 10 Jay Dobies 2009-06-04 14:45:53 EDT
Pitching back to QA. I've installed the latest ISO (May 29) twice locally and have not been able to reproduce it. I also asked jsherrill to give me a second pair of eyes on the code to see if we could find any issues, but everything looked good.

I'm wondering if the equals/hashCode changes from above for some reason weren't in the last QA build this was found on.

If you come across this again, please let me know as soon as you see it so I can take a look at the increased logging on the server.
Comment 11 wes hayutin 2009-06-04 15:20:25 EDT
lol...
finally working in 5/29 ;)
Comment 12 wes hayutin 2009-06-15 14:00:38 EDT
broken again in 6/12.1
Comment 13 Jay Dobies 2009-06-17 16:07:03 EDT
Master
commit	3bb101ec6b46754611762c302647817cb3c105a5
tree	66152108cdcca065b51adf5aa0f07b8d277b7d0e

Vader
commit	273bb0b0e9439a8e3e54bc8a62842ea6843d12db
tree	0de8d1f8b7aa0499b0bf7e574d1c737336843211


java/code/src/com/redhat/rhn/common/security/acl/PackageAclHandler.java
java/code/src/com/redhat/rhn/domain/rhnpackage/PackageFactory.java


Analysis:
These links aren't being displayed because the PackageAclHandler.aclPackageTypeCapable method is returning false when checking the ACL for the pages. This method will return false under the following conditions:

- The ACL does not provide any parameters. Those are defined in a static XML file that was double-checked to make sure the parameters exist. If this was the case, I doubt we'd see the non-deterministic nature of this bug.

- Any of the user, pid, or package retrieved from the DB was null. We wouldn't see the page at all if the user was null. We wouldn't see the package details if the PID or package was null. So I'm pretty confident this isn't the issue.

- The capability listing for the package's architecture is null. This is where I suspect the issue is, so more on this later.

- The capability listing for the architecture is retrieved but does not contain the ACL parameter in question. The entries in the capability listings are hard coded in the Java code, so if these were wrong I doubt we'd see the non-deterministic nature of this bug.

Given the above, I'm leaning towards the issue being in the retrieval of the capability listings for the given package architecture. These listings are returned in a map, where the key used to be (before this commit) the ArchType object itself. Previous attempts to fix this were to introduce the equals/hashCode method to this class, without which that retrieval by object is not guaranteed to be consistent. The lack of these methods was definitely a bug.

Looking further, when the capability map is created the ArchType objects being used as the keys are read from the database and held as static final objects in the PackageFactory class (the point being, they are loaded when the class itself is loaded). While in theory this should work, I have reservations about combining a hibernate retrieval with static constants in this manner.

Further investigation showed this retrieval isn't actually needed. The objects are being retrieved by the arch label which is hard coded in the PackageFactory class. Since this label is available at ACL checking time, the simpler approach is to use the label directly in the capability map. This avoids the initial DB hit to load and cache the ArchType objects (not that the queries were painful) but also makes the map retrieval simpler by simply using strings.

I'm not 100% certain this will fix the bug. However, it does remove one of the more potentially fragile areas of this flow (the database call) and simplifies the map lookup.

While I was in there, I changed the capability map to return sets instead of lists for the capabilities for a (very) minimal performance increase (and to feed my compulsion, a list just felt wrong there).

If this fails again in a future build, please let me know the machine as soon as possible, since restarting the Satellite server seems to make the issue go away and it's getting hard to have any solid time on a box that manifests the bug.
Comment 14 wes hayutin 2009-06-22 10:19:49 EDT
SUCCESS.. looks good :)
6/19 build
Comment 15 Milan Zázrivec 2009-08-04 07:30:31 EDT
Verified in stage -> RELEASE_PENDING
Comment 16 Brandon Perkins 2009-09-10 16:35:31 EDT
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/RHEA-2009-1434.html

Note You need to log in before you can comment on or make changes to this bug.