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

[v2.9] Add new agent TLS chart value & validation jsonSchema #45522

Open
wants to merge 6 commits into
base: release/v2.9
Choose a base branch
from

Conversation

mallardduck
Copy link
Member

@mallardduck mallardduck commented May 16, 2024

Issue:

TODO:

  • Investigate how to handle "upgrades"
  • Verify how the r/r internal code is going to handle this

Problem

Solution

Testing

Engineering Testing

Manual Testing

Automated Testing

  • Test types added/modified:
    • Unit - Helm unit tests added to cover new value and possible results.

Summary: The tests verify all expected results based on potential valid inputs.

QA Testing Considerations

Regressions Considerations

TODO

Existing / newly added automated tests that provide evidence there are no regressions:

  • TODO

Copy link
Contributor

@MbolotSuse MbolotSuse left a comment

Choose a reason for hiding this comment

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

Overall comments:

  • This is missing the logic to maintain system-store in case of upgrades
  • The boolean translation is unnecessary - both the setting and the helm chart value should both be strings
  • Be careful on adding validation to "extraEnv" - since that's an existing value, you need to consider backward campatibility if you change that.

chart/templates/_helpers.tpl Outdated Show resolved Hide resolved
chart/templates/deployment.yaml Outdated Show resolved Hide resolved
chart/values.yaml Outdated Show resolved Hide resolved
chart/values.schema.json Outdated Show resolved Hide resolved
@mallardduck mallardduck marked this pull request as ready for review May 31, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants