Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce logs in webapp container (depends on blackduck-common release, do not merge) #1390

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Reduce logs in webapp container (depends on blackduck-common release, do not merge) #1390

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
475ea88
sysadmin check add in blackduck connectivity checker to pass to black…
zahidblackduck Mar 17, 2025
34ff371
unused import cleanup in blackduck connectivity checker
zahidblackduck Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions ...com/blackduck/integration/detect/lifecycle/boot/product/BlackDuckConnectivityChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@
import com.blackduck.integration.blackduck.service.dataservice.BlackDuckRegistrationService;
import com.blackduck.integration.blackduck.service.dataservice.UserGroupService;
import com.blackduck.integration.blackduck.service.dataservice.UserService;
import com.blackduck.integration.configuration.property.Properties;
import com.blackduck.integration.configuration.property.types.enums.EnumProperty;
import com.blackduck.integration.detect.configuration.DetectProperties;
import com.blackduck.integration.detect.configuration.DetectUserFriendlyException;
import com.blackduck.integration.detect.configuration.enumeration.BlackduckScanMode;
import com.blackduck.integration.detect.configuration.enumeration.ExitCodeType;
import com.blackduck.integration.exception.IntegrationException;
import com.blackduck.integration.log.SilentIntLogger;
Expand All @@ -29,7 +25,7 @@

public class BlackDuckConnectivityChecker {
private static final LinkMultipleResponses<UserGroupView> USERGROUPS = new LinkMultipleResponses<>("usergroups", UserGroupView.class);

private static final String SYS_ADMIN_ROLE = "System Administrator";
private final Logger logger = LoggerFactory.getLogger(this.getClass());

public BlackDuckConnectivityResult determineConnectivity(BlackDuckServerConfig blackDuckServerConfig)
Expand All @@ -51,15 +47,15 @@ public BlackDuckConnectivityResult determineConnectivity(BlackDuckServerConfig b

String version = "";
try {
version = blackDuckRegistrationService.getBlackDuckServerData().getVersion();
UserView userView = userService.findCurrentUser();
UserGroupService userGroupService = blackDuckServicesFactory.createUserGroupService();
List<RoleAssignmentView> roles = userGroupService.getServerRolesForUser(userView);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to be careful about here is pagination.
I'm not sure what the maximum number of roles is, but it may be greater than the default page size.

boolean isAdmin = checkIsAdmin(roles);
version = blackDuckRegistrationService.getBlackDuckServerData(isAdmin).getVersion();
Copy link
Contributor

@andrian-sevastyanov andrian-sevastyanov Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed on a call, the greater intention of passing in isAdmin here is not clear.
To someone working with this code in the future it may look like the only thing this code cares about here is the version.
That could lead a developer down a path of refactoring, not realizing the importance of the side effect, and all of a sudden we will start making extra calls in other places again.

I think Ideally we would:

  1. Determine user roles once
  2. Fetch BlackDuckServerData based on it once
  3. Pass around this data to wherever its needed, including phone home

This is more work but I believe is more reliable and would minimize the number of API calls needed,.

logger.info(String.format("Successfully connected to Black Duck (version %s)!", version));

if (logger.isDebugEnabled()) {
UserView userView = userService.findCurrentUser();
logger.debug("Connected as: " + userView.getUserName());

UserGroupService userGroupService = blackDuckServicesFactory.createUserGroupService();
List<RoleAssignmentView> roles = userGroupService.getServerRolesForUser(userView);
logger.debug("Server Roles: " + roles.stream().map(RoleAssignmentView::getName).distinct().collect(Collectors.joining(", ")));

BlackDuckApiClient blackDuckApiClient = blackDuckServicesFactory.getBlackDuckApiClient();
Expand All @@ -75,4 +71,8 @@ public BlackDuckConnectivityResult determineConnectivity(BlackDuckServerConfig b
}
return BlackDuckConnectivityResult.success(blackDuckServicesFactory, blackDuckServerConfig, version);
}
}

private boolean checkIsAdmin(List<RoleAssignmentView> roles) {
return roles.stream().anyMatch(role -> SYS_ADMIN_ROLE.equals(role.getName()));
}
}