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

Add extraManifests value #27

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Add extraManifests value #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cec1f05
Add extraManifests value
CiucurDaniel Oct 14, 2024
cc0a7cc
Add new chart value to README and schema
CiucurDaniel Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ You can then run `helm search repo zipkin` to see the charts.
| zipkin.storage.elasticsearch.username | string | no default | Basic authentication of X-Pack security |
| zipkin.storage.elasticsearch.password | string | no default | Basic authentication of X-Pack security |
| zipkin.storage.type | string | `"mem"` | |
| extraManifests | list | `[]` | Extra manifests to deploy with the helm chart |



The values are validated using a JSON schema, which contains logic to enforce either:

Expand Down
13 changes: 13 additions & 0 deletions charts/zipkin/ci/extraManifests-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
extraManifests:
- apiVersion: v1
kind: ConfigMap
metadata:
name: some-config
data:
example: true
- apiVersion: v1
kind: ConfigMap
metadata:
name: some-other-config
data:
example: true
4 changes: 4 additions & 0 deletions charts/zipkin/templates/extra-objects.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{{ range .Values.extraManifests }}
---
{{ tpl (toYaml .) $ }}
{{ end }}
3 changes: 3 additions & 0 deletions charts/zipkin/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@
"type": "object"
}
}
},
"extraManifests": {
"type": "object"
}
}
}
9 changes: 9 additions & 0 deletions charts/zipkin/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,12 @@ extraServicePorts: []

# global variables to use in values. typically used in tests.
global: {}

# extra manifests to deploy with the helm chart
extraManifests: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be extraManifests: {} to match the object type?

Copy link
Member

Choose a reason for hiding this comment

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

PR welcome and merged quickly to fix any mistakes! One way even more ideal is to add ci tests as we have for some other options, but I'll leave that up to the person to decide to do or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marcuz no, array is correct. If you want to be more accurate you can change it to be array of objects [ {} ].

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, the fix in #32 seems to work. 😄

Copy link
Member

Choose a reason for hiding this comment

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

thanks @marcuz and @CiucurDaniel. If either of you have time to add a fake value in a ci file, please do, so that the mystery stays solved https://github.com/openzipkin/zipkin-helm/tree/master/charts/zipkin/ci

that or add the linting not currently happening to our workflow config
https://github.com/openzipkin/zipkin-helm/blob/master/.github/workflows/test.yml#L72

# - apiVersion: v1
# kind: ConfigMap
# metadata:
# name: some-config
# data:
# example: true