Skip to content

HPCC-35978 Handle invalid vault definitions#21149

Open
kenrowland wants to merge 4 commits into
hpcc-systems:candidate-10.2.xfrom
kenrowland:HPCC-35978
Open

HPCC-35978 Handle invalid vault definitions#21149
kenrowland wants to merge 4 commits into
hpcc-systems:candidate-10.2.xfrom
kenrowland:HPCC-35978

Conversation

@kenrowland

@kenrowland kenrowland commented Mar 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Improved values.schema.json to validate Akeyless vault definitions
Added common vaults section copy XSL with validation for component XMLs
Added type/kind validation to jsecrets code

Signed-Off-By: Kenneth Rowland kenneth.rowland@lexisnexisrisk.com

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change improves the code (refactor or other change that does not change the functionality)
  • This change fixes warnings (the fix does not alter the functionality or the generated code)
  • This change is a breaking change (fix or feature that will cause existing behavior to change).
  • This change alters the query API (existing queries will have to be recompiled)

Checklist:

  • My code follows the code style of this project.
    • My code does not create any new warnings from compiler, build system, or lint.
  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit is signed.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the CONTRIBUTORS document.
  • The change has been fully tested:
    • I have added tests to cover my changes.
    • All new and existing tests passed.
    • I have checked that this change does not introduce memory leaks.
    • I have used Valgrind or similar tools to check for potential issues.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Cloud-compatibility
    • Premature optimization
    • Existing deployed queries will not be broken
    • This change fixes the problem, not just the symptom
    • The target branch of this pull request is appropriate for such a change.
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised JIRA issues to address them separately
  • This is a user interface / front-end modification
    • I have tested my changes in multiple modern browsers
    • The component(s) render as expected

Smoketest:

  • Send notifications about my Pull Request position in Smoketest queue.
  • Test my draft Pull Request.

Testing:

HPCC-35978 Handle invalid vault definitions
f934886 
Improved values.schema.json to validate Akeyless vault definitions
Added common vaults section copy XSL with validation for component XMLs
Added type/kind validation to jsecrets code

Signed-Off-By: Kenneth Rowland kenneth.rowland@lexisnexisrisk.com
@kenrowland kenrowland changed the base branch from master to candidate-10.2.x March 25, 2026 19:01
@kenrowland kenrowland requested a review from asselitx March 25, 2026 19:02
@github-actions

github-actions Bot commented Mar 25, 2026

Copy link
Copy Markdown

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-35978

Jirabot Action Result:
Workflow Transition To: Merge Pending
Updated PR

asselitx
asselitx requested changes Mar 26, 2026
View reviewed changes

@asselitx asselitx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe two small changes but looks good otherwise.

Comment thread initfiles/componentfiles/configxml/vaults-common.xsl Outdated
<vaults>
<xsl:copy-of select="/Environment/Software/vaults/@*"/>
<xsl:for-each select="/Environment/Software/vaults/*">
<xsl:variable name="vaultType" select="translate(normalize-space(@type), 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz')"/>

@asselitx asselitx Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think vaultType and vaultKind are used here so they could be removed.

@kenrowland kenrowland Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct, an artifact from previous incantations. Good find.

Comment thread initfiles/componentfiles/configxml/vaults-common.xsl Outdated
</xsl:if>
</xsl:template>

</xsl:stylesheet> No newline at end of file

@asselitx asselitx Mar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like a missing final newline

@kenrowland kenrowland Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added

@kenrowland kenrowland requested a review from asselitx March 26, 2026 18:33
@github-actions

github-actions Bot commented Mar 26, 2026

Copy link
Copy Markdown

🔄 Upmerge Test Results

Status: ✅ All branches merged successfully
PR: #21149 - HPCC-35978 Handle invalid vault definitions
Base Branch: candidate-10.2.x
Test Time: 2026-03-26 18:33:37 UTC

✅ Successful Branches (1)

  • master
    This comment was automatically generated by the upmerge test workflow.

@kenrowland

kenrowland commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

@ghalliday @jakesmith Please merge

ghalliday
ghalliday requested changes Apr 8, 2026
View reviewed changes

@ghalliday ghalliday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kenrowland this is failing (probably because of the unused static function.
Please add documentation to the jira to describe what the checks are and why they are there.
Also there should be some tests in testing/helm/errtests to exercise these checks.

Kenneth Rowland added 2 commits April 8, 2026 10:12
Removed static method not called
d0b71a8
Removed unused static method
7c5bcbb 
Added helm positive and negative values tests for Akeyless
@kenrowland

kenrowland commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Removed the unused static method and added positive and negative tests for helm. Interesting that there were no existing Hashicorp tests.

@kenrowland kenrowland requested a review from ghalliday April 8, 2026 15:03
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown

🔄 Upmerge Test Results

Status: ✅ All branches merged successfully
PR: #21149 - HPCC-35978 Handle invalid vault definitions
Base Branch: candidate-10.2.x
Test Time: 2026-04-08 15:22:07 UTC

✅ Successful Branches (1)

  • master
    This comment was automatically generated by the upmerge test workflow.

ghalliday
ghalliday requested changes Apr 10, 2026
View reviewed changes

@ghalliday ghalliday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kenrowland I am confused - why are there two different enums which are now identical?

Comment thread helm/hpcc/values.schema.json
"enum": ["kv-v2", "kv-v1"]
"enum": ["kv-v2", "kv-v1", "akeyless"]
},
"type": {

@ghalliday ghalliday Apr 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we have two enumerations that now contain identical values? It is too late, but the names should have been chosen more carefully. Can type be deleted, and just rely on type?

Comment thread helm/hpcc/values.schema.json
},
"required": ["kind"]
},
"then": {

@ghalliday ghalliday Apr 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is going to cause backward compatibility issues with deployments that do not have the value set - when it was previously optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ghalliday ghalliday ghalliday requested changes

@asselitx asselitx asselitx approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kenrowland @ghalliday @asselitx