[k8s] improve kubernetes_secrets provider secret logging #6841

pkoutsovasilis · 2025-02-13T09:11:26Z

What does this PR do?

This PR enhances the logging of the kubernetes_secrets provider to improve visibility into secret updates and cache operations. The changes include:

Improving cache update logging to specify which secrets were updated or expired.
Enhancing error messages to clarify failures in secret retrieval.
Updating the function fetchFromAPI to return the resource version of a secret.
Adding unit tests for fetchFromAPI to validate secret retrieval behavior.

Why is it important?

Previously, the kubernetes_secrets provider had minimal logging, making it difficult to determine when secrets were updated, expired, or failed to refresh. This lack of visibility led to troubleshooting challenges, especially in cases where secrets did not rotate as expected. With these improvements, users can now track secret updates more effectively without exposing sensitive values.

Checklist

I have read and understood the pull request guidelines of this project.
My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in ./changelog/fragments using the changelog tool
I have added an integration test or an E2E test

Disruptive User Impact

This change does not alter the core functionality of the kubernetes_secrets provider but improves logging. Users should see more detailed logs related to secret updates, which can be useful for debugging. There are no breaking changes expected.

How to test this PR locally

Run the kubernetes_secrets provider unit-tests

Related issues

Closes Make it possible to view and debug the lifecycle of a secret populated by the kubernetes_secrets provider #6187

mergify · 2025-02-13T09:12:04Z

This pull request does not have a backport label. Could you fix it @pkoutsovasilis? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-./d./d is the label that automatically backports to the 8./d branch. /d is the digit
backport-active-all is the label that automatically backports to all active branches.
backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

elasticmachine · 2025-02-14T21:06:16Z

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

swiatekm

LGTM

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go

blakerouse

Overall this looks good, just a question about why the use of backslash when golang offers a way to not have to do that.

changelog/fragments/1739437836-Improve-kubernetes_secrets-provider-secret-logging.yaml

blakerouse

Missed the inline comment.

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go

blakerouse

Thanks!

pkoutsovasilis · 2025-02-17T16:04:33Z

Thanks!

I thank you for the comments 🙂

pkoutsovasilis · 2025-02-17T21:12:20Z

@cmacknz if we follow our policies this shouldn't be backported on any active releases branches. But I believe such enhancement will be helpful for users that use kubernetes_secrets provider on active release such 8.17.x. WDYT? 🙂

cmacknz · 2025-02-19T22:02:28Z

I would backport this everywhere the secrets provider implementation has changed.

cmacknz · 2025-02-19T22:04:07Z

Is there a way we can get the current secret resource version agent substituted into the agent policy added into say computed-config.yaml or something? I suppose we could just log it, that might get verbose if there are a lot of secrets and/or policy changes.

One of the questions I had when I was trying to debug this was, "is the secret provider working correctly but the agent somehow has a stale secret value" and I don't see anything to help me answer that.

pkoutsovasilis · 2025-02-20T17:25:23Z

after looking more into running this PR actually on a k8s cluster to see how variables of kubernetes_secrets provider are printed in the computed-config inside a diagnostics, I run into this issue panic

panic: assignment to entry in nil map

goroutine 145 [running]:
github.com/elastic/elastic-agent-libs/mapstr.M.Put(0x0, {0x40000bbd40, 0x12}, {0x9a8db80, 0x4000444320})
        /Users/pkoutsovasilis/go/pkg/mod/github.com/elastic/elastic-agent-libs@v0.18.2/mapstr/mapstr.go:383 +0x140
github.com/elastic/elastic-agent/internal/pkg/composable.(*controller).Run.func2()
        /Users/pkoutsovasilis/repos/elastic-agent/internal/pkg/composable/controller.go:204 +0x164
created by github.com/elastic/elastic-agent/internal/pkg/composable.(*controller).Run in goroutine 131
        /Users/pkoutsovasilis/repos/elastic-agent/internal/pkg/composable/controller.go:194 +0x440

I don't see how the provider can cause this and I have already reached out to @blakerouse for support

Until then let's keep this PR un-merged

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go

pkoutsovasilis · 2025-02-21T09:27:45Z

So @cmacknz I did get a hands-on experience with it. As of now, you could debug the kubernetes_secrets provider by just downloading the diagnostics and looking at the computed-config.yaml. This will contain the actual secret values where you can deem if the provider has stale secrets or not. Although this might seem somewhat convenient, IMO this is an issue and we should change it. I opened this one as a follow-up.

pkoutsovasilis · 2025-02-21T10:40:30Z

created issue for one new flaky test here #6965

pkoutsovasilis · 2025-02-21T11:31:17Z

created another issue for a new flaky test here

elastic-sonarqube · 2025-02-25T00:05:47Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

elasticmachine · 2025-02-25T02:02:10Z

💚 Build Succeeded

Buildkite Build
Commit: e7cc94a

History

💔 Build #17350 failed efe5679

cc @pkoutsovasilis

pkoutsovasilis · 2025-02-25T12:03:35Z

created one more issue for Flaky one here. Thank you all for the reviews 🤘

* feat: improve kubernetes_secrets provider secret logging * feat: improve readability by replacing Info with Infof * fix: categorise properly the type of change * feat: more logging readability improvements * feat: improve readability by replacing "%s" with %q (cherry picked from commit 0ae5d35)

adamkasztenny · 2025-02-25T15:07:10Z

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go

@@ -151,7 +151,8 @@ func (p *contextProviderK8SSecrets) Fetch(key string) (string, bool) {

 	if p.config.DisableCache {
 		// cache disabled - fetch secret from the API
-		return p.fetchFromAPI(ctx, secretName, secretNamespace, secretKey)
+		val, _, ok := p.fetchFromAPI(ctx, secretName, secretNamespace, secretKey)


@pkoutsovasilis I'm guessing that adding logging here would add too much noise?

this is with cache disabled thus Fetch will always return the latest value from API server, right? we could definitely add it but since there is no intermediate layer that can bear stale secrets I thought we could try and minimise noise. What do you think? 🙂

Would a debug-level log make sense?

adamkasztenny · 2025-02-25T15:11:36Z

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets_test.go

+			secretKey:       "secret_key_not_found",
+		},
+		{
+			name: "key in secret not found",


Isn't this the happy path case, where the key is found?

this is the almost happy path 😆 so the secret is found in the api server, there is no error reading it, but the key inside the secret we are trying to fetch doesn't exist. does it make sense?

I must be missing something 😕 IIUC, the key we're looking for is secret_key (that's what we're passing to the fetchFromAPI function on line 978), and the mock secret we're building also has a key of secret_key (line 974). So to me it seems like the key is found

pkoutsovasilis added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Feb 13, 2025

pkoutsovasilis self-assigned this Feb 13, 2025

feat: improve kubernetes_secrets provider secret logging

205b8f1

pkoutsovasilis force-pushed the k8s/kubernetes_secrets_provider_logging branch from ada501f to 205b8f1 Compare February 13, 2025 09:13

pkoutsovasilis added backport-8.x Automated backport to the 8.x branch with mergify backport-9.0 Automated backport to the 9.0 branch labels Feb 13, 2025

pkoutsovasilis marked this pull request as ready for review February 14, 2025 21:06

pkoutsovasilis requested a review from a team as a code owner February 14, 2025 21:06

pkoutsovasilis requested review from michel-laterman and pchila February 14, 2025 21:06

swiatekm approved these changes Feb 17, 2025

View reviewed changes

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go Outdated Show resolved Hide resolved

feat: improve readability by replacing Info with Infof

14f11a6

blakerouse reviewed Feb 17, 2025

View reviewed changes

changelog/fragments/1739437836-Improve-kubernetes_secrets-provider-secret-logging.yaml Outdated Show resolved Hide resolved

blakerouse reviewed Feb 17, 2025

View reviewed changes

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go Outdated Show resolved Hide resolved

pkoutsovasilis added 2 commits February 17, 2025 17:42

fix: categorise properly the type of change

e2610c9

feat: more logging readability improvements

cc3b045

pkoutsovasilis requested a review from blakerouse February 17, 2025 15:57

blakerouse approved these changes Feb 17, 2025

View reviewed changes

pkoutsovasilis added backport-8.17 Automated backport with mergify backport-8.18 Automated backport to the 8.18 branch labels Feb 20, 2025

michel-laterman approved these changes Feb 20, 2025

View reviewed changes

internal/pkg/composable/providers/kubernetessecrets/kubernetes_secrets.go Outdated Show resolved Hide resolved

feat: improve readability by replacing "%s" with %q

aafbd74

pkoutsovasilis added 2 commits February 24, 2025 13:46

Merge branch 'main' into k8s/kubernetes_secrets_provider_logging

efe5679

Merge branch 'main' into k8s/kubernetes_secrets_provider_logging

e7cc94a

pchila approved these changes Feb 25, 2025

View reviewed changes

pkoutsovasilis merged commit 0ae5d35 into elastic:main Feb 25, 2025
14 checks passed

mergify bot mentioned this pull request Feb 25, 2025

[8.x](backport #6841) [k8s] improve kubernetes_secrets provider secret logging #7005

Open

8 tasks

mergify bot mentioned this pull request Feb 25, 2025

[8.17](backport #6841) [k8s] improve kubernetes_secrets provider secret logging #7006

Open

8 tasks

mergify bot mentioned this pull request Feb 25, 2025

[8.18](backport #6841) [k8s] improve kubernetes_secrets provider secret logging #7007

Open

8 tasks

mergify bot mentioned this pull request Feb 25, 2025

[9.0](backport #6841) [k8s] improve kubernetes_secrets provider secret logging #7008

Open

8 tasks

adamkasztenny reviewed Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[k8s] improve kubernetes_secrets provider secret logging #6841

[k8s] improve kubernetes_secrets provider secret logging #6841

pkoutsovasilis commented Feb 13, 2025 •

edited

Loading

mergify bot commented Feb 13, 2025

elasticmachine commented Feb 14, 2025

swiatekm left a comment

blakerouse left a comment

blakerouse left a comment

blakerouse left a comment

pkoutsovasilis commented Feb 17, 2025

pkoutsovasilis commented Feb 17, 2025

cmacknz commented Feb 19, 2025

cmacknz commented Feb 19, 2025

pkoutsovasilis commented Feb 20, 2025

pkoutsovasilis commented Feb 21, 2025

pkoutsovasilis commented Feb 21, 2025

pkoutsovasilis commented Feb 21, 2025

elastic-sonarqube bot commented Feb 25, 2025

elasticmachine commented Feb 25, 2025 •

edited

Loading

pkoutsovasilis commented Feb 25, 2025 •

edited

Loading

adamkasztenny Feb 25, 2025

pkoutsovasilis Feb 25, 2025

adamkasztenny Feb 25, 2025

adamkasztenny Feb 25, 2025

pkoutsovasilis Feb 25, 2025

adamkasztenny Feb 25, 2025

[k8s] improve kubernetes_secrets provider secret logging #6841

[k8s] improve kubernetes_secrets provider secret logging #6841

Conversation

pkoutsovasilis commented Feb 13, 2025 • edited Loading

What does this PR do?

Why is it important?

Checklist

Disruptive User Impact

How to test this PR locally

Related issues

mergify bot commented Feb 13, 2025

elasticmachine commented Feb 14, 2025

swiatekm left a comment

Choose a reason for hiding this comment

blakerouse left a comment

Choose a reason for hiding this comment

blakerouse left a comment

Choose a reason for hiding this comment

blakerouse left a comment

Choose a reason for hiding this comment

pkoutsovasilis commented Feb 17, 2025

pkoutsovasilis commented Feb 17, 2025

cmacknz commented Feb 19, 2025

cmacknz commented Feb 19, 2025

pkoutsovasilis commented Feb 20, 2025

pkoutsovasilis commented Feb 21, 2025

pkoutsovasilis commented Feb 21, 2025

pkoutsovasilis commented Feb 21, 2025

elastic-sonarqube bot commented Feb 25, 2025

Quality Gate passed

elasticmachine commented Feb 25, 2025 • edited Loading

💚 Build Succeeded

History

pkoutsovasilis commented Feb 25, 2025 • edited Loading

adamkasztenny Feb 25, 2025

Choose a reason for hiding this comment

pkoutsovasilis Feb 25, 2025

Choose a reason for hiding this comment

adamkasztenny Feb 25, 2025

Choose a reason for hiding this comment

adamkasztenny Feb 25, 2025

Choose a reason for hiding this comment

pkoutsovasilis Feb 25, 2025

Choose a reason for hiding this comment

adamkasztenny Feb 25, 2025

Choose a reason for hiding this comment

pkoutsovasilis commented Feb 13, 2025 •

edited

Loading

elasticmachine commented Feb 25, 2025 •

edited

Loading

pkoutsovasilis commented Feb 25, 2025 •

edited

Loading