infisical: support secrets within paths for `data` references by lgo · Pull Request #4305 · external-secrets/external-secrets

lgo · 2025-01-19T10:21:58Z

Problem Statement

When attempting to access a secret in a directory, provided secretsPath: /, like so:

apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: infisical-managed-secrets
spec:
  secretStoreRef:
    kind: SecretStore
    name: infisical
  target:
    name: auth-api
  data:
    - secretKey: API_KEY
      remoteRef:
        key: "foo/bar"

This will always result in a 404 API error from Infisical, because the API request is malformed (/api/v3/secrets/raw/foo/bar). The correct API request would instead be /api/v3/secrets/raw/bar?secretPath=/foo. This either:

Made it so you simply cannot use secrets in folders with direct references via data
Made it so you have to use a separate secret provider with a distinct path, which isn't very usable or very obvious

In combination with #4059, this would result in empty secrets but after that issue is fixed these would fail to sync with an error.

Overall this was also quite confusing with the SecretStore's secretsPath property - would a remoteRef be an additive path? Would it supercede the SecretStore?

Related Issue

Fixes #4298

Proposed Changes

As suggested on the issue (#4298 (comment)) this makes a particular changes:

remoteRef's with paths take precedence over any secretsPath on the SecretStore
If a remoteRef is detected to be a path (i.e. it contains /), an error is raised if it does not start with / with an actionable error indicating to use / and that it is an absolute reference. This should hopefuly further clarify the interaction between this, and secretsPath
Documentation is updated with more specific examples of data in Infisical and how you would retrieve it, now that these features work

I've tested this against a self-hosted Infisical instance. I used a branch rebased off of #4304 to test this (lgo:joey-infisical-folder-handling-testing)

With a remoteRef: {key: "foo/bar"}
❌ ExternalSecret expectedly fails with a secret key referencing a folder must start with a '/' as it is an absolute path, key: foo/bar
With a remoteKey: {key: "/foo/bar"}
✅ the secret successfully syncs.

TODO:

This needs to be rebased after merging infisical: fix error handling which previously failed silently (missing secrets, incorrect auth, etc.) #4304. I had a few changes layered on that I'll include after the rebase.

Checklist

I have read the contribution guidelines
All commits are signed with git commit --signoff
My changes have reasonable test coverage
All tests pass with make test
I ensured my PR is ready for review with make reviewable

akhilmhdh · 2025-01-20T08:09:27Z

Hi @lgo

That was quick. Code wise looking good to me. I'll do the application test today and give my approval on my side.

One small issue is, can we bring back the secret path option. Removing it would be a breaking change and want to keep it backward compatible. Simply keep that option as it is and if user provides it and not defaultSecretPath, the first one will be used. The docs can be kept with new one, may be mentioning secretPath is depreciated would be great. As both option was optional from the start, this should give customers less headache.

lgo · 2025-01-20T08:30:51Z

One small issue is, can we bring back the secret path option. Removing it would be a breaking change and want to keep it backward compatible. Simply keep that option as it is and if user provides it and not defaultSecretPath, the first one will be used. The docs can be kept with new one, may be mentioning secretPath is depreciated would be great. As both option was optional from the start, this should give customers less headache.

Sounds good. We could also just keep secretPath and ensure the documentation is clear. Your suggestion of keeping it around if we do add defaultSecretPath is a much better idea than changing it.

lgo · 2025-01-20T09:32:15Z

I decided to just avoid adding/changing defaultSecretPath and pushed up the changes, a few test fixes, and quality check fixes. This should now be good and just has a few test conflicts with the other PR (which I'll address once it's in).

Skarlso · 2025-01-26T15:56:58Z

@lgo Hello 👋 Could you please sign your commits? :)

lgo · 2025-02-01T16:51:55Z

Done! Sorry, I was away on a trip. I have not yet had a chance to build out the e2e tests either.

Hi @akhilmhdh - just checking if you had any additional testing you'd like to do on your end before checking off?

Skarlso · 2025-02-03T08:34:13Z

Is this now ready for review?

lgo · 2025-02-03T15:52:00Z

Yep!

sonarqubecloud · 2025-02-05T06:40:55Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

pkg/provider/infisical/client.go

pkg/provider/infisical/provider.go

gusfcarvalho · 2025-04-14T11:04:54Z

Hi @lgo ! Were you able to address the points @Skarlso mentioned?

akhilmhdh · 2025-04-15T14:39:50Z

Hi @Skarlso @gusfcarvalho @lgo

I don't have push access to this branch. Can I make the changes requested as another PR with your commits to make this merged, if @lgo doesn't have time to continue working on it? We would love to see this changes getting shipped.

gusfcarvalho · 2025-04-15T18:39:44Z

As long as you’re fork from @lgo repo, so his contributions thus far are correctly accredited to him, I’m up for it 😄

lgo · 2025-04-15T19:09:45Z

Heh, yea - sorry this went stale! I haven't had a lot of spare time lately and I've worked around this by using a custom build that included this patch.

Do feel free to fork this to take it over the finish line (if you don't get to it by EOW, I do have a long weekend that I can likely come back and rebase + address those)

lgo · 2025-04-15T19:12:50Z

(actually, I'll go ahead and handle the rebase right now as I already had a rebased branch and roll in the existing PR feedback just to leave a slightly cleaner slate, whether or not there's further work needed)

lgo · 2025-04-16T15:32:13Z

Okay I've pushed up a rebase, squashed the commits down, and addressed the one feedback. Two things that still need to be done:

Make sure the unit tests are still working. (For some reason go test <file> isn't working and make unit-test is failing but with a large wall of text so I need to revisit that)
Deploy this and step through the test cases I outlined again
Check whether Infisical's API requires a leading / on the secretPath query param (https://infisical.com/docs/api-reference/endpoints/secrets/read#parameter-secret-path). If they do, I'll make this much clearer in the code as I've partially written up error / defensive cases for tihs.

I'll take a pass at this later tonight, or tomorrow morning, and see if I can wrap these items up.

typedrat · 2025-05-11T22:50:58Z

Okay I've pushed up a rebase, squashed the commits down, and addressed the one feedback. Two things that still need to be done:

Make sure the unit tests are still working. (For some reason go test <file> isn't working and make unit-test is failing but with a large wall of text so I need to revisit that)

Deploy this and step through the test cases I outlined again

Check whether Infisical's API requires a leading / on the secretPath query param (https://infisical.com/docs/api-reference/endpoints/secrets/read#parameter-secret-path). If they do, I'll make this much clearer in the code as I've partially written up error / defensive cases for tihs.

I'll take a pass at this later tonight, or tomorrow morning, and see if I can wrap these items up.

Any updates on this? I really prefer using external-secrets over the native Infisical operator, and this would really help make it a lot more usable.

gusfcarvalho · 2025-05-12T02:05:44Z

Hi @typedrat! If you can take the remaining points from these commits and push your contribution to it, I’d gladly review it!

otherwise, chances are this might go stale just because that’s the nature of community driven effort 🤷🏽‍♂️

typedrat · 2025-05-12T20:18:40Z

Hi @typedrat! If you can take the remaining points from these commits and push your contribution to it, I’d gladly review it!

otherwise, chances are this might go stale just because that’s the nature of community driven effort 🤷🏽‍♂️

Fair enough, I know how open source goes haha

I don't know Go and I've only got the barest bit of experience with using Infisical's API, so I don't know if I'm really the one to get this over the finish line myself, but I'll come back and look at this when I'm out of my work crunch if no one else has picked this up.

Skarlso · 2025-05-13T04:43:52Z

@typedrat I'll take a look. :)

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> On-behalf-of: Joey Pereira

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

Skarlso · 2025-05-13T06:07:02Z

I had to rebase to get the signed commit to work, but I left an On-behalf-of in the commit message to make sure to credit OP. :) I also tested with a free account and it seems to work as expected as far as I understand it.

sonarqubecloud · 2025-05-13T06:08:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

Adriien-M · 2025-05-13T09:37:03Z

Thanks @Skarlso for your work 👍

lgo requested a review from a team as a code owner January 19, 2025 10:21

lgo requested a review from knelasevero January 19, 2025 10:21

lgo force-pushed the joey-infisical-folder-handling branch from ab735bc to a2234f0 Compare January 19, 2025 21:10

lgo marked this pull request as draft January 19, 2025 21:22

lgo marked this pull request as ready for review January 20, 2025 00:20

lgo force-pushed the joey-infisical-folder-handling branch 3 times, most recently from b54cfb2 to 5a3da95 Compare January 20, 2025 09:27

lgo force-pushed the joey-infisical-folder-handling branch from fc5f5e2 to dba1c01 Compare February 1, 2025 16:45

Skarlso self-assigned this Feb 3, 2025

Skarlso reviewed Feb 5, 2025

View reviewed changes

pkg/provider/infisical/client.go Outdated Show resolved Hide resolved

Skarlso reviewed Feb 5, 2025

View reviewed changes

pkg/provider/infisical/provider.go Show resolved Hide resolved

lgo force-pushed the joey-infisical-folder-handling branch 3 times, most recently from 7affeca to 844a048 Compare April 16, 2025 15:22

Skarlso force-pushed the joey-infisical-folder-handling branch 3 times, most recently from f3d624c to f989b34 Compare May 13, 2025 05:10

infisical: allow for retrieving secrets within paths

ac8ea21

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> On-behalf-of: Joey Pereira

Skarlso force-pushed the joey-infisical-folder-handling branch from f989b34 to ac8ea21 Compare May 13, 2025 05:12

fixed my faulty index path logic and test asserts

4f26685

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

Skarlso approved these changes May 13, 2025

View reviewed changes

Skarlso merged commit 65a8d4b into external-secrets:main May 13, 2025
20 checks passed

renovate-f882d bot mentioned this pull request Aug 8, 2025

feat(helm): update chart external-secrets ( 0.15.1 → 0.19.2 ) armando-g/home-ops#94

Merged

1 task

dc-tec mentioned this pull request Aug 9, 2025

chore(deps): update helm release external-secrets to v0.20.4 dc-tec/k8s-gitops#160

Open

1 task

samip5-bot bot mentioned this pull request Aug 10, 2025

feat(helm): update chart external-secrets ( 0.16.2 → 0.19.2 ) samip5/k8s-cluster#2216

Merged

1 task

Uh oh!

Conversation

lgo commented Jan 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Related Issue

Proposed Changes

Checklist

Uh oh!

akhilmhdh commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgo commented Jan 20, 2025

Uh oh!

lgo commented Jan 20, 2025

Uh oh!

Skarlso commented Jan 26, 2025

Uh oh!

lgo commented Feb 1, 2025

Uh oh!

Skarlso commented Feb 3, 2025

Uh oh!

lgo commented Feb 3, 2025

Uh oh!

sonarqubecloud bot commented Feb 5, 2025

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

gusfcarvalho commented Apr 14, 2025

Uh oh!

akhilmhdh commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusfcarvalho commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgo commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgo commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgo commented Apr 16, 2025 • edited by Skarlso Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typedrat commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gusfcarvalho commented May 12, 2025

Uh oh!

typedrat commented May 12, 2025

Uh oh!

Skarlso commented May 13, 2025

Uh oh!

Skarlso commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented May 13, 2025

Quality Gate passed

Uh oh!

Uh oh!

Adriien-M commented May 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lgo commented Jan 19, 2025 •

edited

Loading

akhilmhdh commented Jan 20, 2025 •

edited

Loading

akhilmhdh commented Apr 15, 2025 •

edited

Loading

gusfcarvalho commented Apr 15, 2025 •

edited

Loading

lgo commented Apr 15, 2025 •

edited

Loading

lgo commented Apr 15, 2025 •

edited

Loading

lgo commented Apr 16, 2025 •

edited by Skarlso

Loading

typedrat commented May 11, 2025 •

edited

Loading

Skarlso commented May 13, 2025 •

edited

Loading