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: update to using default scrapeclass for tls config #517

Merged
merged 27 commits into from
Jul 11, 2024
Merged

feat: update to using default scrapeclass for tls config #517

merged 27 commits into from
Jul 11, 2024

Conversation

zachariahmiller
Copy link
Contributor

@zachariahmiller zachariahmiller commented Jun 27, 2024 •
edited
Loading

Description

add pod monitors to uds-core operator automation and UDS package CR monitor spec.
update to using default scrapeClass for tls config in prometheus and "exempt" class to override default tls config
update core components existing pod and service monitor implementations to fit with the new default scrapeClass implementation
migrate pepr over to using the generated helm based implementation to facilitate ability to override and align zarf.yaml composition organization with the other packages.
add authorization to the endpoint configuration options for monitors

Related Issue

Fixes #417

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@zachariahmiller zachariahmiller linked an issue Jun 27, 2024 that may be closed by this pull request
Add a way to add podMonitors under the monitor key. #417
Closed
@zachariahmiller zachariahmiller marked this pull request as draft June 27, 2024 22:45
@zachariahmiller
Copy link
Contributor Author

needs docs and there are a couple of things to potentially remove that i left for someone if they wanted to test locally in review. Namely additional resources in the app-tenant test app and commented out values in the pepr chart import components values file that doesnt actually need to be there for things to work. @mjnagel If you want to take a look LMK any feedback.

After changing the behavior for the scrape classes i verified the changes i made to make the existing uds-core monitors match by deploying both from this branch and from main and doing an yaml diff of the prometheus config yaml of both deployments.

@zachariahmiller zachariahmiller changed the title feat: update to using default scrapeclass for tls config, feat: update to using default scrapeclass for tls config Jun 27, 2024
@zachariahmiller
Copy link
Contributor Author

Made an initial attempt at the docs. Not sure how yall want to communicate new vs existing/deprecated behavior format wise especially given that these docs feed into the site.

@zachariahmiller zachariahmiller marked this pull request as ready for review July 3, 2024 21:12
mjnagel
mjnagel requested changes Jul 9, 2024
View reviewed changes
Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

All targets in core show as healthy on a deploy - hopefully most of these comments are small modifications.

docs/configuration/uds-monitoring-metrics.md Outdated Show resolved Hide resolved
src/pepr/operator/reconcilers/package-reconciler.ts Outdated Show resolved Hide resolved
src/pepr/operator/crd/index.ts Outdated Show resolved Hide resolved
src/pepr/operator/crd/sources/package/v1alpha1.ts Show resolved Hide resolved
src/pepr/operator/controllers/monitoring/pod-monitor.ts Outdated Show resolved Hide resolved
src/pepr/zarf.yaml Outdated Show resolved Hide resolved
src/test/app-tenant.yaml Show resolved Hide resolved
@zachariahmiller
Copy link
Contributor Author

@mjnagel I believe i resolved all your comments. Please take a look again when you get a chance.

mjnagel
mjnagel previously approved these changes Jul 11, 2024
View reviewed changes
@mjnagel mjnagel dismissed their stale review July 11, 2024 18:53

Dismissing approval temporarily - metrics-server seems to have an issue...investigating.

mjnagel
mjnagel previously approved these changes Jul 11, 2024
View reviewed changes
@mjnagel mjnagel dismissed their stale review July 11, 2024 19:32

Failing CI, investigating.

mjnagel
mjnagel reviewed Jul 11, 2024
View reviewed changes
tasks/create.yaml Outdated Show resolved Hide resolved
mjnagel
mjnagel approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

LGTM for real this time.

@rjferguson21 rjferguson21 merged commit 258bb6b into main Jul 11, 2024
13 checks passed
@rjferguson21 rjferguson21 deleted the add_pod_monitor_and_scrape_class branch July 11, 2024 21:06
@github-actions github-actions bot mentioned this pull request Jul 11, 2024
Merged
@mjnagel mjnagel mentioned this pull request Jul 12, 2024
Merged
5 tasks
mjnagel added a commit that referenced this pull request Jul 12, 2024
@mjnagel
fix: podmonitor mTLS mutations (#566)
eb613e1 
## Description

This PR has a few follow-on changes for
#517:
- Slightly reworded/ordered documentation to focus on current setup
rather than deprecated setup
- Updated code to support `podSelector` or `selector` for pod monitor
generation
- Updated mutation logic for servicemonitors to:
  - Combine conditionals for exemption
  - Add a new `uds/skip-mutate` annotation for exemption
  - Clarify deprecation of tlsConfig mutation (not scheme)
- Added mutation logic for podmonitors to:
- Allow for exempting podmonitors from default scrape class via
annotation or injection detection
  - Mutate `scheme` to `https` if not exempted
- Switched core podmonitors to use `uds/skip-mutate` annotation rather
than explicit scrape class
- Deleted example test monitoring from other test-apps and enabled pod +
svc monitor for podinfo (since it exposes a real metrics endpoint)

## Related Issue

No issue opened as this PR was just merged.

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed
mjnagel pushed a commit that referenced this pull request Jul 12, 2024
@github-actions
chore(main): release 0.24.0 (#550)
fb365cf 
🤖 I have created a release *beep* *boop*
---


##
[0.24.0](v0.23.0...v0.24.0)
(2024-07-12)


### ⚠ BREAKING CHANGES

* set istio passthrough gateway as optional component
(#547)

### Features

* add unicorn flavor to uds-core
([#507](#507))
([a412581](a412581))
* added standalone dns service for loki
([#548](#548))
([e2efdf9](e2efdf9))
* enable authservice integration
([#201](#201))
([1d4df64](1d4df64))
* set istio passthrough gateway as optional component
(#547)
([e1cab61](e1cab61))
* update to using default scrapeclass for tls config
([#517](#517))
([258bb6b](258bb6b))


### Bug Fixes

* decouple `devMode` and postgres egress
([#554](#554))
([1a98779](1a98779))
* grafana logout not working in some environments
([#559](#559))
([ccb9d9e](ccb9d9e))
* initial creation of child logging
([#533](#533))
([00a5140](00a5140))
* podmonitor mTLS mutations
([#566](#566))
([eb613e1](eb613e1))


### Miscellaneous

* add util function for purging orphans
([#565](#565))
([e84229a](e84229a))
* allow istio proxy injection in zarf ignored namespaces
(#513)
([8921b58](8921b58))
* **deps:** update githubactions upload-artifact to v4.3.4
([#543](#543))
([20889f2](20889f2))
* **deps:** update grafana helm chart to v8.3.2
([#542](#542))
([8ec260c](8ec260c))
* **deps:** update pepr dependencies (jest, uds-common)
([#537](#537))
([547c0bf](547c0bf))
* **deps:** update promtail helm chart to v6.16.3
([#538](#538))
([48b3fea](48b3fea))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjferguson21 rjferguson21 rjferguson21 approved these changes

@mjnagel mjnagel mjnagel approved these changes

@UnicornChance UnicornChance UnicornChance approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a way to add podMonitors under the monitor key.
5 participants
@zachariahmiller @rjferguson21 @mjnagel @UnicornChance @Racer159