feat: add principals flag to cli for ssh dynamic secrets#238
feat: add principals flag to cli for ssh dynamic secrets#238saifsmailbox98 wants to merge 1 commit into
Conversation
|
💬 Discussion in Slack: #pr-review-cli-238-feat-add-principals-flag-to-cli-for-ssh-dynamic-secrets Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if match { | ||
| log.Debug().Msgf("[cache]: found matching active lease: [project=%s], [env=%s], [path=%s], [slug=%s]", |
There was a problem hiding this comment.
don't we also need to add the principals here? isn't it good for debug?
| dynamicSecretManager.RegisterTemplateUnsafe(projectSlug, envSlug, secretPath, slug, templateId, ttl, principals) | ||
|
|
||
| etagData := fmt.Sprintf("%s-%s-%s-%s-%s", projectSlug, envSlug, secretPath, slug, ttl) | ||
| etagData := fmt.Sprintf("%s-%s-%s-%s-%s-%s", projectSlug, envSlug, secretPath, slug, ttl, principals) |
There was a problem hiding this comment.
I think for the cache we won't have a problem if the principals are different, but can't this be a problem on the etag as well? if they change the order of the principals, this would cause a difference an we would write on disk the change (which in case wouldn't really be a change, since only the order was changed)
Not sure if this is a problem, I just wanted to raise this so we can discuss.
| if principals != "" { | ||
| var parsedPrincipals []string | ||
| for _, p := range strings.Split(principals, ",") { | ||
| trimmed := strings.TrimSpace(p) | ||
| if trimmed != "" { | ||
| parsedPrincipals = append(parsedPrincipals, trimmed) | ||
| } | ||
| } | ||
| if len(parsedPrincipals) > 0 { | ||
| leaseConfig["principals"] = parsedPrincipals | ||
| } | ||
| } |
There was a problem hiding this comment.
maybe this could be a helper? if we have something similar in another command we might also need something similar.
2 participants
Description 📣
Adds a
--principalsflag to infisical dynamic-secrets lease create so users can specify which principals to include in SSH dynamic secret leases. Also adds principals support to the agent dynamicSecret template function as an optional 6th argument.Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets