Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented May 14, 2025

What changes were proposed in this pull request?

This PR aims to elevate the quality and maintainability of Helm Chart by introducing a layer of strict validation for user-provided configurations.

Why are the changes needed?

By providing values.schema.json file, we can define and validate the expected configuration values that a user can provide to customize a chart's deployment. This will help the users by preventing mistakes and improving reliability.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the BUILD label May 14, 2025
@dongjoon-hyun
Copy link
Member Author

cc @jiangzho

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-52118] Enforce Helm Chart configuration value verification [SPARK-52118] Enforce Helm Chart configuration value verification May 14, 2025
@dongjoon-hyun
Copy link
Member Author

Could you review this PR, @peter-toth ?

},
"digest": {
"type": "string",
"description": "Docker image digest, takes precedence over tag if set"
Copy link
Member

Choose a reason for hiding this comment

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

tag is required above. But when digest is provided, we don't need tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

If both are set, tag will be ignored semantically, but tag field is still required.

Copy link
Contributor

@peter-toth peter-toth left a comment

Choose a reason for hiding this comment

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

Looks good with the current chart:

% helm lint
==> Linting .

1 chart(s) linted, 0 chart(s) failed

But checked with an invalid one as well:

% helm lint
==> Linting .
[ERROR] values.yaml: - image: tag is required

[ERROR] templates/: values don't meet the specifications of the schema(s) in the following chart(s):
spark-kubernetes-operator:
- image: tag is required


Error: 1 chart(s) linted, 1 chart(s) failed

@dongjoon-hyun
Copy link
Member Author

Thank you, @viirya and @peter-toth !
Merged to main.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-52118 branch May 14, 2025 13:02
dongjoon-hyun added a commit that referenced this pull request May 14, 2025
### What changes were proposed in this pull request?

This PR aims to change `Helm Chart` version from `0.2.0-SNAPSHOT` to `1.0.0-dev` and revise the description as a part of official release of this chart `1.0.0`.

### Why are the changes needed?

Like [Apache Airflow example](https://github.com/apache/airflow/blob/2d56d2626c84089f156a56137647498a6ad2ee7a/chart/Chart.yaml#L20-L24), `Helm Chart` version is independently managed from the application versions (which is Apache Spark K8s Operator).

```yaml
apiVersion: v2
name: airflow
version: 1.17.0-dev
appVersion: 3.0.0
description: The official Helm chart to deploy Apache Airflow, a platform to
  programmatically author, schedule, and monitor workflows
```

As a part of the following series of Helm Chart improvement, I believe we are ready to set `1.0.0` for Helm Chart itself at next release, Apache Spark K8s Operator v0.2.0.
- #190
- #196
- #191
- #194

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass the CIs and manual review.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #197 from dongjoon-hyun/SPARK-52133.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants