Fix NPE on adding new columns in the tables

[harikrishna-patnala]

Description

This PR fixes an NPE in case during the development or any other case when there is an extra column in the tables that are not there in the corresponding VO.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

[harikrishna-patnala]

@blueorangutan package

[blueorangutan]

@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.23%. Comparing base (a4b1a27) to head (572c53d).
⚠️ Report is 5 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...c/main/java/com/cloud/utils/db/GenericDaoBase.java	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #12464   +/-   ##
=========================================
  Coverage     16.23%   16.23%           
  Complexity    13380    13380           
=========================================
  Files          5657     5657           
  Lines        499035   499043    +8     
  Branches      60567    60567           
=========================================
+ Hits          81029    81041   +12     
+ Misses       408969   408963    -6     
- Partials       9037     9039    +2     

Flag	Coverage Δ
uitests	4.03% <ø> (ø)
unittests	17.09% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 16417

[DaanHoogland]

@harikrishna-patnala , I don’t see any of the possible NPEs in the old code being mitigated in the new code. Am I missing something?

[harikrishna-patnala]

@harikrishna-patnala , I don’t see any of the possible NPEs in the old code being mitigated in the new code. Am I missing something?

This is happening during any development or when an extra column was added in the table and no reference in the VO. Here few safe checks are added for that.

[harikrishna-patnala]

@harikrishna-patnala , I don’t see any of the possible NPEs in the old code being mitigated in the new code. Am I missing something?

This is happening during any development or when an extra column was added in the table and no reference in the VO. Here few safe checks are added for that.

@DaanHoogland unfortunately I could not reproduce this now, as the column count seems to be fine in most of the cases, but I think this has caused issues in retrieving the attr object when the sql statement usage contains "SELECT *". In remaining cases where specific columns in SELECT query are used, there won't be this issue. I feel this check can be kept as we are rightly logging when attr is not found.

Please let me know your thoughts.

[DaanHoogland]

Tnx @harikrishna-patnala , I think this can be white-box tested by adding a column to a table in “production”. Some operators may do that to try out and or filter things or aggregate data. If we know how this NPE occurred in the wild it will be easy to simulate in a test env.

[harikrishna-patnala]

Tnx @harikrishna-patnala , I think this can be white-box tested by adding a column to a table in “production”. Some operators may do that to try out and or filter things or aggregate data. If we know how this NPE occurred in the wild it will be easy to simulate in a test env.

I've tried couple of operations with mshost, configuration, account, domain tables but the tested operations didnt hit the "select *" query. Currently we don't have the information of last failure happened.

[DaanHoogland]

Tnx @harikrishna-patnala , I think this can be white-box tested by adding a column to a table in “production”. Some operators may do that to try out and or filter things or aggregate data. If we know how this NPE occurred in the wild it will be easy to simulate in a test env.

I've tried couple of operations with mshost, configuration, account, domain tables but the tested operations didnt hit the "select *" query. Currently we don't have the information of last failure happened.

ok, too bad, but under the credo of defensive programming I think we can merge this anyway.