Add openshift resource detector for distribution: openshift#2330
Add openshift resource detector for distribution: openshift#2330
Conversation
| resourceMetrics: | ||
| - resource: | ||
| attributes: | ||
| - key: cloud.platform |
There was a problem hiding this comment.
ec2 detector resources are missing in the test cluster because openshift has restriction on IMDS access originating from non-host network. User can run the clusterReceiver in hostNetwork or they can give IAM permission to the pod.
| - name: "customfield2" | ||
| value: "customvalue2" | ||
|
|
||
| clusterName: ci-k8s-cluster |
There was a problem hiding this comment.
removed to test the cluster name is retrieved correctly
| - key: cloud.platform | ||
| value: | ||
| stringValue: abcd | ||
| stringValue: aws_eks |
There was a problem hiding this comment.
I excluded this attribute from the normalization done in test; this is a good attr value to ascertain that the order of detectors are correct
03af54c to
0ab49a5
Compare
There was a problem hiding this comment.
Pull request overview
Enables OpenShift-aware resource detection when distribution=openshift and fixes AWS non-EKS resource detection by switching from the eks detector to ec2, ensuring cloud/host attributes are populated correctly across more cluster types.
Changes:
- Add OpenShift (
openshift) and non-EKS AWS (ec2) resource detection configuration and update cluster-name auto-detection logic. - Extend OpenShift RBAC to allow reading
config.openshift.ioinfrastructure resources needed for detection. - Update functional test normalization and expected resource attribute fixtures to reflect stable
cloud.platformvalues and OpenShift/AWS attributes.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| helm-charts/splunk-otel-collector/values.yaml | Updates clusterName guidance to include OpenShift auto-discovery behavior. |
| helm-charts/splunk-otel-collector/values.schema.json | Makes clusterName optional for openshift distributions via schema conditional. |
| helm-charts/splunk-otel-collector/templates/config/_common.tpl | Enables openshift detector and adds ec2 detector for non-EKS AWS; configures resource attributes for both. |
| helm-charts/splunk-otel-collector/templates/clusterRole.yaml | Adds OpenShift config.openshift.io RBAC permissions (guarded by distribution=openshift). |
| helm-charts/splunk-otel-collector/templates/_helpers.tpl | Makes clusterName optional for OpenShift and broadens non-EKS-on-AWS detection logic. |
| functional_tests/functional/testdata/values/rosa_upgrade_from_previous_release_values.yaml | Updates ROSA upgrade test values (clusterName). |
| functional_tests/functional/testdata/values/rosa_test_values.yaml.tmpl | Removes explicit ROSA clusterName to exercise auto-detection. |
| functional_tests/functional/testdata/expected_rosa_values/expected_resource_attributes_cluster_receiver.yaml | Updates expected ROSA cluster-receiver resource attributes (OpenShift cloud attributes + cluster name). |
| functional_tests/functional/testdata/expected_rosa_values/expected_resource_attributes_agent.yaml | Updates expected ROSA agent resource attributes (adds EC2 host/cloud metadata + OpenShift cloud.platform). |
| functional_tests/functional/testdata/expected_gke_values/expected_resource_attributes_cluster_receiver.yaml | Updates expected cloud.platform value for GKE. |
| functional_tests/functional/testdata/expected_gke_values/expected_resource_attributes_agent.yaml | Updates expected cloud.platform value for GKE. |
| functional_tests/functional/testdata/expected_eks_values/expected_resource_attributes_cluster_receiver.yaml | Updates expected cloud.platform value for EKS. |
| functional_tests/functional/testdata/expected_eks_values/expected_resource_attributes_agent.yaml | Updates expected cloud.platform value for EKS. |
| functional_tests/functional/testdata/expected_eks_auto_mode_values/expected_resource_attributes_cluster_receiver.yaml | Updates expected cloud.platform value for EKS auto-mode. |
| functional_tests/functional/testdata/expected_eks_auto_mode_values/expected_resource_attributes_agent.yaml | Updates expected cloud.platform value for EKS auto-mode. |
| functional_tests/functional/testdata/expected_aks_values/expected_resource_attributes_cluster_receiver.yaml | Updates expected cloud.platform value for AKS. |
| functional_tests/functional/testdata/expected_aks_values/expected_resource_attributes_agent.yaml | Updates expected cloud.platform value for AKS. |
| functional_tests/functional/functional_test.go | Preserves cloud.platform during normalization to assert exact platform values. |
| examples/distribution-openshift/rendered_manifests/deployment-cluster-receiver.yaml | Updates rendered manifest checksum due to config changes. |
| examples/distribution-openshift/rendered_manifests/daemonset.yaml | Updates rendered manifest checksum due to config changes. |
| examples/distribution-openshift/rendered_manifests/configmap-cluster-receiver.yaml | Shows openshift detector enabled and configured in rendered output. |
| examples/distribution-openshift/rendered_manifests/configmap-agent.yaml | Shows openshift detector enabled and configured in rendered output. |
| examples/distribution-openshift/rendered_manifests/clusterRole.yaml | Updates rendered ClusterRole to include new OpenShift RBAC rules. |
| .chloggen/openshift-resource-detection.yaml | Changelog entry for OpenShift resource detection enhancement. |
| .chloggen/fix-non-eks-aws-ec2-detector.yaml | Changelog entry for non-EKS AWS detection bug fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b92df4c to
e638844
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enable the openshift resource detection processor when distribution is set to openshift. This provides cloud.provider, cloud.platform, cloud.region, and k8s.cluster.name attributes via the OpenShift API (no IMDS dependency). - Add openshift detector and resource_attributes config in _common.tpl - Add config.openshift.io RBAC permissions to clusterRole - Fix bug where non-EKS clusters on AWS would not have cloud attributes set
6462034 to
dc4252f
Compare
dc4252f to
e3f7ecd
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description:
openshiftresource detection processor when distribution is set toopenshift. This providescloud.provider,cloud.platform,cloud.region, andk8s.cluster.nameattributes via the OpenShift API.eksdetector for non-EKS AWS clusters. However, theeksdetector internally checks for EKS-specific signals (IRSA/Pod Identity token paths, the OIDC issuer URL, and the cluster version string containing -eks-). On non-EKS clusters these checks fail and the detector returns empty. EC2 metadata (host.id,cloud.account.id,cloud.availability_zone, etc.) was never collected. Switched to theec2detector, which works on any AWS instance with IMDS access.Fix schema to not allow
eks/fargatewith emptyclusterName. The previous schema check was only looking foreks*which would match eks/fargate but the eks detector is not added in fargate.Adds unittests for resourcedetectors are set as expected for all combinations of cloudProvider + distribution allowed, and check pipelines which have resourcedetection processors