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

fix: Ensure ServiceMonitor.spec.endpoints[0].port is a string #241

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

connyay
Copy link

@connyay connyay commented Jan 3, 2025

Failing to pass kubeconform checks internally

Description of the change

forces port to be a string.

before this change kubeconform failed with the following error:

stdin - ServiceMonitor backstage is invalid: problem validating schema. Check JSON formatting: jsonschema: '/spec/endpoints/0/port' does not validate with https://artifactory.ci.corp.tanium.com/artifactory/github/datreeio/CRDs-catalog/raw/main/monitoring.coreos.com/servicemonitor_v1.json#/properties/spec/properties/endpoints/items/properties/port/type: expected string, but got number

Existing or Associated Issue(s)

Additional Information

https://github.com/yannh/kubeconform

Checklist

  • Chart version bumped in Chart.yaml according to semver.
  • Variables are documented in the values.yaml and added to the README.md. The helm-docs utility can be used to generate the necessary content. Use helm-docs --dry-run to preview the content.
  • JSON Schema generated.
  • List tests pass for Chart using the Chart Testing tool and the ct lint command.

@connyay connyay requested a review from a team as a code owner January 3, 2025 18:26
@connyay
Copy link
Author

connyay commented Jan 3, 2025

Relates to #228

Failing to pass kubeconform checks internally

Signed-off-by: Connor Hindley <connor.hindley@tanium.com>
Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

@github-actions github-actions bot added the stale label Jan 11, 2025
@ChrisJBurns
Copy link
Contributor

ChrisJBurns commented Jan 11, 2025

@connyay Cheers for the PR. Any chance you could add some tests for the ServiceMonitor? We must have missed them when adding it initially, this should help us avoid any issues going forward. Some examples can be found here

@github-actions github-actions bot removed the stale label Jan 12, 2025
@connyay connyay marked this pull request as draft January 13, 2025 15:30
@connyay
Copy link
Author

connyay commented Jan 13, 2025

Yup yup. Will do.

More from our findings - if metrics.serviceMonitor.port is an integer it should really make it to a targetPort property on the service monitor.

Will work on some tests for this.

@cursedcoder
Copy link

same with ArgoCD:

Failed sync attempt to bcb3d89bbd5cd5dc05f675c263dbcfe841486b00: one or more objects failed to apply, reason: error when patching "/dev/shm/3414896706": ServiceMonitor.monitoring.coreos.com "backstage" is invalid: spec.endpoints[0].port: Invalid value: "integer": spec.endpoints[0].port in body must be of type string: "integer"

currently this means OTEL integration is broken for some users

Copy link

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution!

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.

3 participants