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.

Sign up for GitHub

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

Jump to bottom

feat(chart): automate helm json schema #5075

Merged
merged 29 commits into from
Feb 15, 2025
Merged

feat(chart): automate helm json schema #5075

merged 29 commits into from
Feb 15, 2025

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Feb 9, 2025 •
edited
Loading

Description

Added script and documented how to automatically update json schema. In follow-up could add/update github actions as well.

Relates #4989 but not resolves

Fixes #4860

  • JSON schema generation
    • regenerated schema
  • Helm docs generation
  • Dev-guide
    • how to generate JSON schema
    • how to generate docs
    • how to update CHANGELOG.md

Checklist

  • Unit tests updated
  • End user documentation updated

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 9, 2025
@ivankatliarchuk
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 9, 2025
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
47e3291 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
600c6f9 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
09cb4d0 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
39a9058 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
e1b9ba7 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
e3d18fa 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
51d062b 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
687aeea 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
d87a53e 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
3aa5d0d 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
9b56fa0 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
1cf0de0 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
helm(json-schema): simplified schema validation and documentation gen…
5736225 
…eration

Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk ivankatliarchuk changed the title helm(json-schema): simplified schema validation helm(json-schema): helm json schema enchancement Feb 9, 2025
@mloiseleur
Copy link
Contributor

Wow, I like that PR ❤️ !
I'll let @stevehipwell review it, at a first glance it looks good.

For the github action, there is one ready to go for failing on diff:
https://github.com/losisin/helm-values-schema-json-action?tab=readme-ov-file#fail-on-diff

@mloiseleur
Copy link
Contributor

/retitle feat(chart): automate helm schema

@k8s-ci-robot k8s-ci-robot changed the title helm(json-schema): helm json schema enchancement feat(chart): automate helm schema Feb 9, 2025
@mloiseleur
Copy link
Contributor

/retitle feat(chart): automate helm json schema

@k8s-ci-robot k8s-ci-robot changed the title feat(chart): automate helm schema feat(chart): automate helm json schema Feb 9, 2025
stevehipwell
stevehipwell suggested changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ivankatliarchuk, this looks great but I have a couple of changes/questions.

  • Could you please move the schema values file as where it's been added will trigger an additional chart testing job. If you add a new schema dir under the chart and add the values there that'd work. You should also add this dir to .helmignore, I think ci should also have been added so it'd be worth adding that too.
  • Could you explain why you've added helm-docs type comments for values with defaults?

@ivankatliarchuk
Copy link
Contributor Author

Understood, it's a breaking change pretty-much. Let me know when to rebase

@stevehipwell
Copy link
Contributor

@ivankatliarchuk once #5094 has been approved and merged we can get this in.

@stevehipwell
Copy link
Contributor

@ivankatliarchuk we should be good now.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2025
@ivankatliarchuk
feat(chart): automate helm json schema
9816a64 
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
Merge remote-tracking branch 'refs/remotes/origin/helm-4989' into hel…
23dfe2c 
…m-4989

* refs/remotes/origin/helm-4989:
  Apply suggestions from code review
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2025
@ivankatliarchuk
Copy link
Contributor Author

Rebased, looks like no issues

@stevehipwell
Copy link
Contributor

Thanks @ivankatliarchuk 🥳🎉!!
/lgtm

@mloiseleur can you approve?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2025
@ivankatliarchuk
Copy link
Contributor Author

/lgtm

cc: @mloiseleur

@k8s-ci-robot
Copy link
Contributor

@ivankatliarchuk: you cannot LGTM your own PR.

In response to this:

/lgtm

cc: @mloiseleur

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mloiseleur
mloiseleur reviewed Feb 15, 2025
View reviewed changes
@ivankatliarchuk @mloiseleur
Apply suggestions from code review
a8bf8fe 
Co-authored-by: Michel Loiseleur <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2025
mloiseleur
mloiseleur approved these changes Feb 15, 2025
View reviewed changes
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur, stevehipwell

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2025
@k8s-ci-robot k8s-ci-robot merged commit 9251e55 into kubernetes-sigs:master Feb 15, 2025
14 checks passed
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Feb 16, 2025
@ivankatliarchuk
Merge branch 'master' into feat-aws-tags-batching
93054a5 
* master: (33 commits)
  docs(proposal): support multiple replicas with leader election (kubernetes-sigs#5051)
  feat(chart): automate helm json schema (kubernetes-sigs#5075)
  test(aws): introduce first fixture-based (kubernetes-sigs#5092)
  chore(makefile): add helper and document targets
  feat: Updated chart for v1.15.2 release
  chore(makefile): add helper and document targets
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(deps): bump the dev-dependencies group across 1 directory with 21 updates
  test(domain-filter): simple filters on domain exclusion (kubernetes-sigs#5064)
  chore(deps): bump nosborn/github-action-markdown-cli
  ci(docs): add markdown linters and editorconfig (kubernetes-sigs#5055)
  Address PR comments
  docs: update and refactor contribution part (kubernetes-sigs#5073)
  fix(chart): update rbac for F5 transportserver source (kubernetes-sigs#5066)
  fix(chart): non-string types on svcaccount annotations (kubernetes-sigs#5067)
  Update aws.md
  fix: expand tabs in ClouDNS entry
  fix alignment
  ...
@ivankatliarchuk ivankatliarchuk deleted the helm-4989 branch February 16, 2025 14:28
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Feb 18, 2025
@ivankatliarchuk
Merge branch 'master' into chore-update-reviewers
24c0056 
* master: (97 commits)
  chore(formatting): fix infected files with correct formatting (kubernetes-sigs#5099)
  docs: Fix managed-record-type argument
  docs(proposal): support multiple replicas with leader election (kubernetes-sigs#5051)
  feat(chart): automate helm json schema (kubernetes-sigs#5075)
  docs(proposal): update proposal template, add statuses
  test(aws): introduce first fixture-based (kubernetes-sigs#5092)
  chore(makefile): add helper and document targets
  feat: Updated chart for v1.15.2 release
  chore(makefile): add helper and document targets
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(deps): bump the dev-dependencies group across 1 directory with 21 updates
  test(domain-filter): simple filters on domain exclusion (kubernetes-sigs#5064)
  chore(deps): bump nosborn/github-action-markdown-cli
  ci(docs): add markdown linters and editorconfig (kubernetes-sigs#5055)
  Address PR comments
  docs: update and refactor contribution part (kubernetes-sigs#5073)
  fix(chart): update rbac for F5 transportserver source (kubernetes-sigs#5066)
  fix(chart): non-string types on svcaccount annotations (kubernetes-sigs#5067)
  ...
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Feb 19, 2025
@ivankatliarchuk
Merge branch 'master' into docs-proposal-ipv6-internal-node-ip-rollback
4e4642f 
* master: (31 commits)
  fix(source): debug log on gateway target detection
  Update docs/sources/service.md
  chore(formatting): fix infected files with correct formatting (kubernetes-sigs#5099)
  docs: Fix managed-record-type argument
  Update docs/sources/service.md
  docs(proposal): support multiple replicas with leader election (kubernetes-sigs#5051)
  fixed golangci-lint config
  updated MD files
  updated MD files
  feat(chart): automate helm json schema (kubernetes-sigs#5075)
  docs(proposal): update proposal template, add statuses
  test(aws): introduce first fixture-based (kubernetes-sigs#5092)
  chore(makefile): add helper and document targets
  feat: Updated chart for v1.15.2 release
  chore(makefile): add helper and document targets
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(deps): bump the dev-dependencies group across 1 directory with 21 updates
  update service.md, service.go
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevehipwell stevehipwell stevehipwell approved these changes

@mloiseleur mloiseleur mloiseleur approved these changes

@szuecs szuecs Awaiting requested review from szuecs

Assignees

@stevehipwell stevehipwell

@mloiseleur mloiseleur

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Helm chart values schema is missing an entry for the serviceMonitor property.
4 participants
@ivankatliarchuk @mloiseleur @stevehipwell @k8s-ci-robot