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.

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

[COST-5034] - Use json_query for OCP on Azure sql tag matching #5110

Merged
merged 2 commits into from
May 20, 2024

Conversation

lcouzens
Copy link
Contributor

@lcouzens lcouzens commented May 16, 2024

Jira Ticket

COST-5034

Description

This change will tweak our special case tag matching during OCP on Azure trino SQL to avoid false positives on generic string matching.

Testing

Manually run some trino sql:

  1. create table azure_tag_test (tags varchar, matched_tag varchar);
  2. insert into azure_tag_test (tags, matched_tag) values ('{"environment":"test","namespace":"fake-namespace"}', '');
  3. create table ocp_name_test (namespace varchar);
  4. insert into ocp_name_test (namespace) values ('test');
  5. Try to match the data!: SELECT azure.tags, ocp.namespace, ((strpos(lower(azure.tags), 'namespace') != 0 AND strpos(lower(azure.tags), lower(ocp.namespace)) != 0)) AS match FROM azure_tag_test AS azure CROSS JOIN ocp_name_test AS ocp;
    tags | namespace | match
    -------------------------------------------------------------------------------------------------------+-----------+-------
    {"environment":"test","namespace":"fake-namespace"} | test | true

The above (OLD method) will match fine

Whereas the below (New) method fails to match (expected)

SELECT azure.tags, ocp.namespace, (json_query(azure.tags, 'strict $.namespace' OMIT QUOTES) = ocp.namespace) AS match FROM azure_tag_test AS azure CROSS JOIN ocp_name_test AS ocp;

{"environment":"test","namespace":"fake-namespace"} | test | false

You could also test a correct match by inserting the following:
insert into ocp_name_test (namespace) values ('fake-namespace');
Then run the above sql again

Release Notes

  • proposed release note
* [COST-5034](https://issues.redhat.com/browse/COST-5034) Fix ocp on azure tag matching SQL

@lcouzens lcouzens added the azure-smoke-tests pr_check will build the image and run azure + ocp on azure smoke tests label May 16, 2024
Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.2%. Comparing base (edc3db6) to head (6bab225).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5110     +/-   ##
=======================================
- Coverage   94.2%   94.2%   -0.0%     
=======================================
  Files        378     378             
  Lines      31624   31624             
  Branches    3756    3756             
=======================================
- Hits       29781   29779      -2     
- Misses      1176    1177      +1     
- Partials     667     668      +1     

@lcouzens lcouzens marked this pull request as ready for review May 20, 2024 14:17
@lcouzens lcouzens requested review from a team as code owners May 20, 2024 14:17
@lcouzens lcouzens merged commit bbd7b20 into main May 20, 2024
11 checks passed
@lcouzens lcouzens deleted the COST-5034 branch May 20, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
azure-smoke-tests pr_check will build the image and run azure + ocp on azure smoke tests smokes-required
Projects
None yet
2 participants