MGMT-19297: Error when querying /events endpoint with ?host_ids=uuid1…

[rivkyrizel]

Fixes an SQL ambiguity issue when querying the /events endpoint with host_ids using RHSSO authentication. The ambiguity arises due to joins between infra_envs, hosts, and clusters, causing column name conflicts (e.g., user_name, org_id).

This change ensures the authorization check is applied after the main query by wrapping it in a subquery, preventing conflicts while maintaining access control. Also includes a test case to verify event retrieval by host ID when org tenancy is disabled.

List all the issues related to this PR

• [ ] New Feature
• [ ] Enhancement
• [x] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [x] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [ ] No tests needed

Checklist

• [x] Title and description added to both, commit and PR.
• [x] Relevant issues have been associated (see CONTRIBUTING guide)
• [x] This change does not require a documentation update (docstring, docs, README, etc)
• [x] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

[openshift-ci-robot]

@rivkyrizel: This pull request references MGMT-19297 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.19.0" version, but no target version was set.

In response to this:

Fixes an SQL ambiguity issue when querying the /events endpoint with host_ids using RHSSO authentication. The ambiguity arises due to joins between infra_envs, hosts, and clusters, causing column name conflicts (e.g., user_name, org_id).

This change ensures the authorization check is applied after the main query by wrapping it in a subquery, preventing conflicts while maintaining access control. Also includes a test case to verify event retrieval by host ID when org tenancy is disabled.

List all the issues related to this PR

• [ ] New Feature
• [ ] Enhancement
• [x] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [x] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [ ] No tests needed

Checklist

• [x] Title and description added to both, commit and PR.
• [x] Relevant issues have been associated (see CONTRIBUTING guide)
• [x] This change does not require a documentation update (docstring, docs, README, etc)
• [x] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.78%. Comparing base (1f52e95) to head (2d7e1f1). Report is 16 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7445      +/-   ##
==========================================
+ Coverage   67.26%   68.78%   +1.52%     
==========================================
  Files         335      335              
  Lines       42621    44719    +2098     
==========================================
+ Hits        28669    30760    +2091     
+ Misses      11360    11316      -44     
- Partials     2592     2643      +51     

Files with missing lines	Coverage Δ
internal/events/event.go	84.50% <100.00%> (ø)

... and 10 files with indirect coverage changes

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gamli75]

@rccrdpccl please review

[rccrdpccl]

@rivkyrizel can you please add an example of how the final query looked like before, and how it looks like now? How would this ambiguity generate (i.e. why joinable cluster,infraenv or host would have different org_id or user_name)? Could you expand on the issues that this ambiguity would cause?

[rivkyrizel]

@rccrdpccl The issue was caused by ambiguity between user_name and org_id, which exist in infra_envs, hosts, and clusters. The previous query failed due to SQL ambiguity:

SELECT events.*, infra_envs.user_name, infra_envs.org_id
FROM events
INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id
INNER JOIN hosts ON hosts.id = events.host_id
WHERE hosts.deleted_at IS NULL AND user_name = ?

We now apply filtering in an outer query:

SELECT * FROM (
    SELECT events.*, infra_envs.user_name, infra_envs.org_id
    FROM events
    INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id
    INNER JOIN hosts ON hosts.id = events.host_id
    WHERE hosts.deleted_at IS NULL
) AS s WHERE s.user_name = 'foo' OR s.org_id = 'bar'

Since user_name and org_id exist in multiple tables, they may hold different values due to updates, ownership changes, or historical data inconsistencies.

Resulting Error:

{ 
  "code": "500",  
  "reason": "ERROR: column reference \"user_name\" is ambiguous (SQLSTATE 42702)"  
}

Using a subquery ensures unique column references before filtering, resolving the issue.

[rccrdpccl]

@rivkyrizel thank you. Just nitpicking, but would it be easy to generate a query avoiding a subquery with gorm? Possibly adding the org and username filter on the relevant joins and not on the where clause?

If gorm would make it too convoluted or force us to a manual query we can leave out

[rccrdpccl]

/lgtm

[rccrdpccl]

/approve

[rccrdpccl]

/retest

[gamli75]

/override ci/prow/okd-scos-e2e-aws-ovn

[openshift-ci[bot]]

@gamli75: Overrode contexts on behalf of gamli75: ci/prow/okd-scos-e2e-aws-ovn

In response to this:

/override ci/prow/okd-scos-e2e-aws-ovn

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[gamli75]

/retest

[rivkyrizel]

/retest

[rccrdpccl]

/retest

[rivkyrizel]

/override ci/prow/okd-scos-e2e-aws-ovn

[openshift-ci[bot]]

@rivkyrizel: rivkyrizel unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers.

In response to this:

/override ci/prow/okd-scos-e2e-aws-ovn

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[gamli75]

/override ci/prow/okd-scos-e2e-aws-ovn

[gamli75]

/test edge-e2e-metal-assisted-4-19

[openshift-ci[bot]]

@gamli75: Overrode contexts on behalf of gamli75: ci/prow/okd-scos-e2e-aws-ovn

In response to this:

/override ci/prow/okd-scos-e2e-aws-ovn

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[rccrdpccl]

/lgtm /approve

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rccrdpccl, rivkyrizel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [rccrdpccl]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 91dc61d08376e46710321d2c9dadf09fe263cfd9 and 2 for PR HEAD 2d7e1f1b6693322d4f0c608023cc1aa129f40f20 in total

[openshift-ci[bot]]

@rivkyrizel: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[openshift-bot]

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server This PR has been included in build ose-agent-installer-api-server-container-v4.20.0-202505130412.p0.gb16fd19.assembly.stream.el9. All builds following this will include this PR.