Skip to content

feat: secrets management skill - #60

Open
agaetep wants to merge 23 commits into
mainfrom
agaete/secrets-management
Open

feat: secrets management skill#60
agaetep wants to merge 23 commits into
mainfrom
agaete/secrets-management

Conversation

@agaetep

@agaetep agaetep commented Jun 5, 2026

Copy link
Copy Markdown

Adds a new Claude Code skill lfx-v2-secrets/SKILL.md that guides an agent through wiring up secrets for LFX V2 services using External Secrets Operator + IRSA on AWS.

Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI balanced review requested due to automatic review settings June 5, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new lfx-v2-secrets skill documenting the standard workflow for managing LFX V2 microservice secrets via External Secrets Operator (ESO) + IRSA on AWS, covering both full initial setup and incremental secret additions.

Changes:

  • Introduces a new lfx-v2-secrets/SKILL.md with two operational modes (new service vs. existing service).
  • Documents end-to-end steps spanning lfx-v2-opentofu, lfx-secrets-management, service Helm chart changes, and lfx-v2-argocd.
  • Provides templates, verification checklists, and common troubleshooting workflows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md
agaetep added 2 commits June 5, 2026 16:44
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 5, 2026 20:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
detjensrobert
detjensrobert previously approved these changes Jun 5, 2026

@detjensrobert detjensrobert left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good!

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
dealako and others added 2 commits June 10, 2026 09:34
Copilot AI review requested due to automatic review settings June 15, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 8 comments.

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Copilot AI review requested due to automatic review settings June 15, 2026 21:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
agaetep added 2 commits June 15, 2026 14:39
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 21:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.

Comment thread skills/lfx-v2-secrets/SKILL.md
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md
Comment thread skills/lfx-v2-secrets/SKILL.md
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
agaetep added 2 commits June 15, 2026 14:55
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings June 15, 2026 22:12
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 4, 2026 17:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

skills/lfx-v2-secrets/SKILL.md:50

  • This rule is not fixed across all sources: the Auth0 templates use auth0/<ClientName_With_Underscores>, while 1Password uses <third-party>/<service>, and line 438 separately says every path must include the service. Define source-specific path patterns consistently so the agent does not rewrite a valid Auth0 path.
| AWS Secrets Manager path pattern | `<3rd-party-service>/<lfx-v2-service-name>/[<secret_type>]` |

skills/lfx-v2-secrets/SKILL.md:118

  • Requiring the lfx-v2- prefix rejects lfx-self-serve, even though Step 5 explicitly supports that service. Define <service> as the exact repository/deployment name and describe lfx-v2- as the normal convention rather than an invariant.
Identify `<service>` — the fully qualified service name including the `lfx-v2-` prefix (e.g.,
`lfx-v2-committee-service`). If the user did not include it in their request, ask for it now
before proceeding. This is used directly in all resource names: K8s Secret is `<service>-secrets`,
role ARN ends in `<service>`, etc.

skills/lfx-v2-secrets/SKILL.md:218

  • File presence is not a sufficient readiness gate. This skips setup without checking the IAM definition, chart values, or per-environment IRSA annotations; conversely, one missing file sends the agent through every Step 3 substep and can recreate resources that already exist. Validate each required component's contents and run only the missing substeps.
**If all three files are present** → skip to [Step 4](#step-4-add-entry-to-lfx-secrets-management).

**If any are missing** → continue with Step 3 to set up the infrastructure first.

skills/lfx-v2-secrets/SKILL.md:44

  • This universal name conflicts with the explicit lfx-self-serve exception (pcc-secrets) at lines 548-554, and the checklist repeats the universal rule at line 606. State the exception in both places; otherwise verification can incorrectly replace the working pcc-secrets reference.

This issue also appears on line 50 of the same file.

| K8s Secret name | `<service>-secrets` (e.g., `lfx-v2-committee-service-secrets`) |

skills/lfx-v2-secrets/SKILL.md:202

  • A 404 from raw.githubusercontent.com does not prove that a file is absent: private or inaccessible repositories also return 404. The referenced ArgoCD/OpenTofu repositories are not visible through unauthenticated GitHub access, so this check can classify existing infrastructure as missing. Use authenticated gh api or a local checkout, and treat raw 404 as inconclusive until repository access is verified.

This issue also appears on line 216 of the same file.

Fetch these three URLs (a 404 means the file doesn't exist yet):

skills/lfx-v2-secrets/SKILL.md:230

  • The cross-repo sequence belongs centrally, but Steps 3-5 inline implementation templates owned by lfx-v2-opentofu, each service chart, lfx-v2-argocd, and lfx-secrets-management. This duplicates repo-owned truth contrary to skills/lfx/references/deployment-routing.md:10-12 and repo-map.md:263-282, so these templates will drift. Keep orchestration here and direct the agent to each owning repo's runtime docs/current manifests for exact schemas.
### Step 3a: Add IAM Service Account Entry in `lfx-v2-opentofu`

In the [lfx-v2-opentofu](https://github.com/linuxfoundation/lfx-v2-opentofu) repo,
edit `iam-service-account-definitions.yaml` and add:

skills/lfx-v2-secrets/SKILL.md:672

  • A SecretStore is an ESO custom resource and is never mounted into a pod, so pod events cannot show this. Pod events can reveal a missing Kubernetes Secret/key; ESO synchronization must be diagnosed from ExternalSecret and SecretStore status before controller logs and AWS checks.
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 4, 2026 19:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Suppressed comments (7)

skills/lfx-v2-secrets/SKILL.md:402

  • Step 1 asks which environments need the secret, but this template—and both Auth0 templates—always deploys to all three; the tip at line 447 reinforces that behavior. A development-only request would therefore publish to staging and production. Parameterize envs and the 1Password vault map from the environments selected in Step 1.
  envs: [development, staging, production]

skills/lfx-v2-secrets/SKILL.md:674

  • A SecretStore is not mounted into an application pod; ESO uses it to materialize a Kubernetes Secret. This check sends debugging in the wrong direction. Pod events can reveal a missing Secret/key, while SecretStore readiness must be inspected on the custom resource.
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted

skills/lfx-v2-secrets/SKILL.md:10

  • The bare trigger “IRSA configuration” overlaps lfx-object-store-ops, whose description explicitly fires for “IRSA for S3”. A storage prompt can therefore load two incompatible workflows. Narrow this trigger to External Secrets IRSA and explicitly exclude S3/object-storage provisioning.
  "set up secrets", "wire up External Secret Operator", "add a secret to this service", "IRSA configuration",

skills/lfx-v2-secrets/SKILL.md:125

  • This requirement excludes the lfx-self-serve service that the skill explicitly supports later, and the claim that <service> is used directly in all resource names conflicts with its pcc-secrets/pcc exceptions. Treat the service as a lookup key and verify derived names instead.
Identify `<service>` — the fully qualified service name including the `lfx-v2-` prefix (e.g.,
`lfx-v2-committee-service`). If the user did not include it in their request, ask for it now
before proceeding. This is used directly in all resource names: K8s Secret is `<service>-secrets`,
role ARN ends in `<service>`, etc.

skills/lfx-v2-secrets/SKILL.md:239

  • The skill copies exact implementation schemas owned by lfx-v2-opentofu, service charts, lfx-v2-argocd, and lfx-secrets-management (also at lines 262–368 and 399–503). That violates this repository’s fanout boundary and makes central instructions drift whenever an owner changes its schema. Keep the cross-repo sequence here, but point agents to each owning repository’s runtime documentation/examples for the actual YAML.
```yaml

skills/lfx-v2-secrets/SKILL.md:498

  • These placeholders contradict the earlier rule that Auth0 renames are automatic and must always use the auth0_ prefix. An agent can emit arbitrary or unresolved names here. Use the prescribed names consistently in both rename_fields and field_types.
        client_id: <renamed_client_id>
        client_public_key: <renamed_client_public_key>
        client_private_key: <renamed_client_private_key>

skills/lfx-v2-secrets/SKILL.md:668

  • This workflow modifies lfx-secrets-management in step 3 but tells the agent to submit only an ArgoCD PR, leaving the new sync entry unreviewed and undeployed. Submit both changed repositories, as the preceding existing-service workflow does.
7. Submit argocd PR

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated

@dealako dealako left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @agaetep 👋 — thanks for the thorough turnaround on this. You cleared all 7 items from the last round, and the new local-testing section is a genuinely valuable addition.

👏 Nice work

  • allowed-tools now includes Write, Edit — the blocker is gone.
  • The local-dev plaintext-secrets concern is not just patched but properly designed away: the new Local Testing Before Committing section resolves values at load time via op item get (nothing written to disk), and Step 5 stays reference-only via secretKeyRef.
  • The tags: vs eso_service_tag contradiction, the auth0_clients.yml reference mismatch, the verification-checklist per-env gap, the bare code fences (MD040 now clean), and the e.g.+etc nit are all fixed. Clean sweep.

A few new items surfaced in the new commits — one is a real safety blocker introduced/elevated by this revision.

Issue count (open items)

  • 🔴 Blocking: 1 — Step 2b treats a raw-URL 404 as "file doesn't exist", but lfx-v2-argocd / lfx-secrets-management / lfx-v2-opentofu are private and return 404 unauthenticated. I verified this empirically. With Write/Edit now granted, an already-configured service gets misclassified as greenfield and the agent re-runs the destructive Step 3, overwriting live ESO manifests.
  • 🟡 Minor: 4 — Option B's "repeat the pair per field" hits AlreadyExists on the 2nd field; the Step 2b skip-gate is all-or-nothing and ignores the 2a IAM entry (partial-setup overwrite); the mechanical ESOExternal Secret Operator expansion introduced product-name errors (singular vs. the official plural External Secrets Operator, a self-referential frontmatter gloss, and "External Secret Operator operator"); markdownlint MD013 now fails on 7 lines.
  • Nit: 2.env "must be gitignored" is stated passively (have the agent verify/append the rule); the reworded tip "the name of the fully qualified service name" is redundant/vague.

Reconciliation with Copilot

I independently agree with the three most recent Copilot comments and have consolidated them here: the private-repo 404 misclassification (Copilot @ line 221), the multi-field kubectl create secret AlreadyExists bug (line 99), and the skip-gate ignoring the IAM entry (line 225). The earlier Copilot note about the Helm default true on automountServiceAccountToken was already addressed in 0cf73d6.

Decision

🔴 Needs changes before approval — great progress and nearly there; the private-repo 404 handling is the one that genuinely must change before this ships, since it can drive destructive edits to production. The rest is quick polish.

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 5, 2026 16:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

skills/lfx-v2-secrets/SKILL.md:254

  • The “all four present” gate omits configuration that Step 3 creates: the chart’s values.yaml.serviceAccount block and every per-environment IRSA annotation in Step 3d. A repository can have all four checked files while either configuration is absent, causing this branch to skip setup and leave ESO unable to create/use the annotated ServiceAccount. Add these configurations as independent Step 2 checks and gate Steps 3b/3d on their actual state.
**Evaluate each of the four items independently** — the 2a IAM entry, the Helm
`serviceaccount.yaml`, `SecretStore.yaml`, and `ExternalSecret.yaml`:

- **Present** → leave it alone; do not re-run that item's Step 3 sub-step.
- **Missing** → run only that item's Step 3 sub-step (3a for the IAM entry, 3b for the
  ServiceAccount, 3c for SecretStore/ExternalSecret).

skills/lfx-v2-secrets/SKILL.md:333

  • This central skill inlines the exact ArgoCD manifests, service-chart template/defaults, and secrets-management schema even though the ownership references assign those implementation surfaces to their respective repos (repo-map.md:267-272, contract-ownership.md:70-72). That duplicates fast-changing implementation truth in a 736-line always-loaded skill. Keep the cross-repo sequence here, but have agents read the owning repos’ current docs/files for exact templates and validation.
### Step 3c: Create Custom Resources in `lfx-v2-argocd`

The `SecretStore` and `ExternalSecret` are **static YAML files** (not Helm templates) placed in
`lfx-v2-argocd/custom-resources/<service>/`. Create only the file(s) 2b found missing — if one
already exists, leave it as-is.

skills/lfx-v2-secrets/SKILL.md:494

  • The fixed auth0_client_id/auth0_client_secret renames are only unique when a service has one Auth0 client. This workflow explicitly supports BFF and M2M clients for the same service, and the generated ExternalSecret merges all tagged JSON with conflictPolicy: Error; a second client therefore creates duplicate keys and stops synchronization. Include a client-purpose component in each renamed field and carry those names through field_types and environment wiring.
> For LFX V2 services, always rename fields to a descriptive name prefixed with `auth0_`
> (e.g. `auth0_client_id`, `auth0_client_secret`) so keys are unambiguous in the merged K8s Secret.

skills/lfx-v2-secrets/SKILL.md:716

  • A SecretStore is an ESO custom resource; it is never mounted into an application pod. Pod events can reveal that the generated Kubernetes Secret or one of its keys is missing, but this check cannot report whether the SecretStore is “mounted.”
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted

skills/lfx-v2-secrets/SKILL.md:721

  • In this architecture ESO—not the application pod—calls the AWS Secrets Manager API. Testing the pod’s AWS connectivity can pass or fail independently of synchronization and sends debugging to the wrong workload; check connectivity from the external-secrets controller instead.
6. **Topology/firewalling** — pod can reach AWS API endpoint (check SecurityGroup, NACL, DNS)

skills/lfx-v2-secrets/SKILL.md:10

  • The standalone “IRSA configuration” trigger overlaps the existing object-storage skill, whose description explicitly fires on “IRSA for S3.” A prompt about S3 IRSA can therefore load this secrets workflow instead of the owning provisioning workflow. Scope this trigger to Secrets Manager/ESO and add an explicit object-storage exclusion.

This issue also appears on line 329 of the same file.

  objects exist before deciding which steps to run. Use this skill whenever someone says
  "set up secrets", "wire up ESO", "add a secret to this service", "IRSA configuration",
  "External Secrets for V2", or any mention of AWS Secrets Manager integration with
  Kubernetes for LFX V2 services.

skills/lfx-v2-secrets/SKILL.md:39

  • “Fixed across all V2 services” is contradicted by this same skill: Auth0 uses auth0/<ClientName_With_Underscores>, while lfx-self-serve uses pcc-secrets rather than <service>-secrets. Treat these as defaults and identify the source/service-specific exceptions; otherwise an agent can generate the wrong path or Secret name before reaching the later caveats.

This issue also appears on line 492 of the same file.

These values are fixed and apply across all V2 services:

skills/lfx-v2-secrets/SKILL.md:242

  • This bash block contains only a URL for the ServiceAccount check, so running it produces a shell “command not found” rather than an HTTP status that distinguishes present from missing. Use an actual authenticated/raw fetch command and test its exit status.

This issue also appears in the following locations of the same file:

  • line 716
  • line 721
# ServiceAccount in the service Helm chart (public repo — raw fetch is fine; 404 means missing)
https://raw.githubusercontent.com/linuxfoundation/<service>/main/charts/<service>/templates/serviceaccount.yaml

Comment thread skills/lfx-v2-secrets/SKILL.md
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 5, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (8)

skills/lfx-v2-secrets/SKILL.md:18

  • This 747-line body is loaded in full on every trigger. Repo review guidance requires a skill approaching several hundred lines to use progressive disclosure; carrying all templates, examples, reference implementations, troubleshooting, and communication guidance here adds substantial context before the agent can act. Keep the decision flow in SKILL.md and move the detailed procedures into purpose-specific references/ files with explicit read conditions.
# LFX V2 Secrets Setup Guide

skills/lfx-v2-secrets/SKILL.md:218

  • The fallback omits service_account, even though lines 224-227 require its actual value for both the Helm resource and SecretStore reference. If the inaccessible existing entry overrides this field, the workflow silently uses <service> and produces an AWS authentication failure. Ask for all three values.
If this fails for a reason other than "file/entry not found" (e.g., auth error), ask the user to
provide the service's namespace and eso_service_tag.

skills/lfx-v2-secrets/SKILL.md:407

  • Step 2 never checks the per-environment IRSA annotations, so this sub-step cannot follow the stated “missing items only” gate. When the four checked objects exist, line 260 skips Step 3 and a missing role annotation is never repaired; when any other item is missing, 3d runs even if its annotations already exist. Add an independent annotation check for each selected environment and gate 3d on those results.
### Step 3d: Add IRSA Annotation in `lfx-v2-argocd` Per-Environment Values

In `values/dev/<service>.yaml` (repeat for staging and prod with the matching account ID):

skills/lfx-v2-secrets/SKILL.md:541

  • These placeholders contradict lines 151-153, which say Auth0 field renames are automatic, always use the auth0_ prefix, and must not be requested from the user. Leaving the JWT names unspecified forces the agent to invent them and can break the fixed environment keys expected later. Use the prescribed names consistently here and in field_types.
      rename_fields:
        client_id: <renamed_client_id>
        client_public_key: <renamed_client_public_key>
        client_private_key: <renamed_client_private_key>

skills/lfx-v2-secrets/SKILL.md:451

  • This template hardcodes all three environments despite Step 1 explicitly collecting a subset. The Auth0 templates repeat the same issue at lines 509 and 533, and line 497 tells the agent to sync all three, so a development-only request would deploy credentials to staging and production. Parameterize envs from the user's selection and include only the corresponding vault mappings.
  envs: [development, staging, production]

skills/lfx-v2-secrets/SKILL.md:10

  • The trigger phrase IRSA configuration is broader than secrets and overlaps /lfx-skills:lfx-object-store-ops, whose description explicitly fires for S3 IRSA. This would load the secrets workflow for unrelated object-storage work. Narrow this to ESO/Secrets Manager IRSA and add the negative trigger.

This issue also appears on line 18 of the same file.

  "set up secrets", "wire up ESO", "add a secret to this service", "IRSA configuration",
  "External Secrets for V2", or any mention of AWS Secrets Manager integration with
  Kubernetes for LFX V2 services.

skills/lfx-v2-secrets/SKILL.md:431

  • This unconditionally routes agents to implement changes in the private secrets-management repo, but the canonical routing rule says source definitions, paths, tags, and rotation are a DevOps/CloudOps handoff and must not be routed there by default (skills/lfx/references/deployment-routing.md:92-97). Gate direct edits on an explicit request plus confirmed access; otherwise hand the prepared requirements to Platform Engineering.
In the [lfx-secrets-management](https://github.com/linuxfoundation/lfx-secrets-management) repo,
add an entry for each secret to `secrets/lfx/<service>.yml` — one file per LFX V2 service.
If the file doesn't exist yet, create it. If it already exists, append the new entry.

skills/lfx-v2-secrets/SKILL.md:732

  • A SecretStore is not mounted into the application pod, and AWS requests are made by the ESO controller rather than that pod. These checks therefore send debugging to the wrong events and network path. Inspect pod events only for the generated Secret/key, then inspect the ExternalSecret/SecretStore status and ESO controller connectivity.
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted
2. **ESO logs** — `kubectl logs -n external-secrets-system deployment/external-secrets`
3. **AWS Secrets Manager permissions** — verify IRSA role has `SecretsManager:GetSecretValue` on the path
4. **Secret exists in AWS Secrets Manager** — lfx-secrets-management automation has synced the secret
5. **ExternalSecret status** — `kubectl describe externalsecret <name>` shows sync status

Comment thread skills/lfx-v2-secrets/SKILL.md Outdated
Comment thread skills/lfx-v2-secrets/SKILL.md
Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 6, 2026 15:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (8)

skills/lfx-v2-secrets/SKILL.md:10

  • The generic "IRSA configuration" trigger overlaps /lfx-skills:lfx-object-store-ops, whose frontmatter explicitly triggers on IRSA for S3. An object-storage provisioning prompt can therefore load this Secrets Manager workflow. Restrict this trigger to ESO/Secrets Manager and explicitly exclude S3/object-storage IRSA.
  objects exist before deciding which steps to run. Use this skill whenever someone says
  "set up secrets", "wire up ESO", "add a secret to this service", "IRSA configuration",
  "External Secrets for V2", or any mention of AWS Secrets Manager integration with
  Kubernetes for LFX V2 services.

skills/lfx-v2-secrets/SKILL.md:18

  • This 799-line body is loaded in full on every trigger. The local-testing variants, large YAML templates, verification checklist, examples, and debugging guide are independently consumable detail, so keeping all of them inline creates avoidable context cost and makes repo-owned procedures harder to maintain. Keep the routing/decision flow in SKILL.md and move those sections into focused references/ files with explicit read conditions.
# LFX V2 Secrets Setup Guide

skills/lfx-v2-secrets/SKILL.md:296

  • The false/absent customResources case is left broken: line 287 correctly says the new CRs would be inert, but this branch proceeds directly to create them. A new standard-pattern service can complete the workflow without ever deploying its SecretStore or ExternalSecret. Enable the flag in every target environment before Step 3c.
**If neither signal fires** → the absence is real; proceed to Step 3c as normal.

skills/lfx-v2-secrets/SKILL.md:540

  • Step 5 never writes remoteRef.key; it only adds workload secretKeyRef entries, and the standard tag-discovery ExternalSecret has no remoteRef at all. Referring the agent to a nonexistent field in Step 5 makes the path instruction impossible to follow. Limit this advice to CLI/debugging uses (or explicitly to chart-owned, data-list ExternalSecrets).
>   writing `remoteRef.key` in Step 5, running `aws secretsmanager describe-secret`, or
>   debugging a failed ExternalSecret sync — the relative `path:` will not resolve there.

skills/lfx-v2-secrets/SKILL.md:589

  • Earlier instructions mandate automatic auth0_ prefixes and explicitly say not to ask the user for rename values, but this JWT template reintroduces unspecified rename placeholders. An agent cannot deterministically fill this template under those rules. Use the concrete auth0_client_* keys here, matching the non-JWT template and Step 1.
        client_id: <renamed_client_id>
        client_public_key: <renamed_client_public_key>
        client_private_key: <renamed_client_private_key>

skills/lfx-v2-secrets/SKILL.md:750

  • This concrete workflow contradicts Step 2c and the referenced service's current chart: lfx-v2-invite-service already contains chart-owned templates/secretstore.yaml and templates/externalsecret.yaml. The required outcome is to stop and report the deviation, not proceed through the standard Step 3; doing so risks a second ESO setup. Replace this with a verified standard-pattern service or rewrite it as a chart-owned workflow.
3. Check for existing ESO objects — none found, proceed with Step 3
4. Follow Steps 3–5 in order

skills/lfx-v2-secrets/SKILL.md:773

  • Step 3 creates an lfx-secrets-management change, but this workflow only submits the ArgoCD PR. Without submitting and merging the sync-entry PR, the coordinated deployment in step 4 has nothing to deploy.
7. Submit argocd PR

skills/lfx-v2-secrets/SKILL.md:784

  • A SecretStore is ESO controller configuration; it is never mounted into the workload pod, so pod events cannot report whether it mounted. Likewise, the consuming pod does not call AWS—ESO does—so the network check on line 784 targets the wrong component. Diagnose missing Secret/key events on the pod, then inspect the ExternalSecret and ESO controller connectivity.
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted

Address remaining review comments from copilot-pull-request-reviewer on PR #60:

- skills/lfx-v2-secrets/SKILL.md: added an exception note to the Key
  Constants table flagging that lfx-self-serve uses pcc-secrets/service-pcc
  instead of the universal <service>-secrets/service-<eso_service_tag>
  pattern, with a pointer to the Step 5 detail.
- skills/lfx-v2-secrets/SKILL.md: clarified the AWS Secrets Manager path
  tips to explain when to append /<secret_type> to keep paths unique for
  services with more than one secret from the same 3rd-party provider.

The remaining 48 unresolved threads on this PR were already addressed by
prior commits on this branch; those threads are being resolved with
comments pointing to the specific text that already covers them.

Resolves 50 review threads.

Signed-off-by: Antonia Gaete <agaete@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 7, 2026 18:20
@agaetep

agaetep commented Aug 7, 2026

Copy link
Copy Markdown
Author

Review Feedback Addressed

Commit: 47065cd

Cross-checked all 50 unresolved review threads (copilot-pull-request-reviewer, and human review from dealako) against the current state of skills/lfx-v2-secrets/SKILL.md.

No Change Needed (already fixed by prior commits on this branch)

48 threads flagged issues that earlier commits on this branch had already resolved (the GitHub threads were simply never marked resolved). This covered:

  • automountServiceAccountToken placement in the ServiceAccount template
  • <service> consistently meaning the fully-qualified service name (no lfx-v2-lfx-v2-... doubling risk)
  • SecretStore name constant matching the static SecretStore.yaml pattern
  • eso_service_tag used consistently for AWS resource tags (no hardcoded mismatches)
  • envs: key used consistently across 1Password/Auth0 templates
  • allowed-tools frontmatter includes Write, Edit
  • Skill file discoverable at skills/lfx-v2-secrets/SKILL.md
  • AWS Secrets Manager path pattern generalized (no stale cloud/... prefix)
  • Auth0 JWT reference pointing at the correct per-service file
  • tags: list vs eso_service_tag kept distinct, with the lfx-self-serve exception documented
  • Verification checklist covering the per-environment values alternative
  • Markdownlint (MD040 bare fences, MD013 line length) clean
  • Redundant phrasing cleanup
  • kubectl create secret using stdin/process substitution and a single multi-field invocation
  • Step 2 gating evaluating all four infra items independently (no all-or-nothing overwrite risk)
  • Private-repo 404 handling via authenticated gh api checks
  • Correct "External Secrets Operator" product naming
  • .env gitignore check as an explicit imperative step
  • service_account defaulting used consistently in ServiceAccount/SecretStore templates

Replied to each of those 48 threads pointing at the specific text in the current file that addresses it.

Changes Made

  • skills/lfx-v2-secrets/SKILL.md: added an exception note under the Key Constants table flagging that lfx-self-serve uses pcc-secrets/service-pcc instead of the universal <service>-secrets/service-<eso_service_tag> pattern, with a pointer to the Step 5 detail (flagged by copilot-pull-request-reviewer).
  • skills/lfx-v2-secrets/SKILL.md: clarified the AWS Secrets Manager path tips to explain appending /<secret_type> when a service has more than one secret from the same 3rd-party provider, resolving a path-collision gap (flagged by copilot-pull-request-reviewer).

Threads Resolved

50 of 50 unresolved threads addressed and resolved in this iteration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (9)

skills/lfx-v2-secrets/SKILL.md:18

  • At 805 lines, this skill loads every template, example, debugging guide, and communication rule on every trigger. That defeats progressive disclosure and makes cross-repo implementation details more likely to drift. Keep the routing/decision flow in SKILL.md, and move local testing, repository-specific templates, verification, and debugging into focused references/ files with explicit instructions for when to read each one.
# LFX V2 Secrets Setup Guide

skills/lfx-v2-secrets/SKILL.md:50

  • This is presented as a fixed path pattern for all V2 secrets, but both Auth0 templates use auth0/<ClientName_With_Underscores> instead of including the LFX service name. An agent cannot tell which rule wins. Distinguish the 1Password and Auth0 path patterns here.
| AWS Secrets Manager path pattern | `<3rd-party-service>/<lfx-v2-service-name>/[<secret_type>]` |

skills/lfx-v2-secrets/SKILL.md:100

  • op item get terminates field output with a newline, and kubectl --from-file preserves those bytes. The locally created Secret therefore contains a trailing newline that Option A and the eventual JSON-backed Secret do not, so credentials can fail only in this test path. Strip only trailing command-output newlines before feeding the process-substitution file.
  --from-file=<field_name_1>=<(op item get "<1Password item name>" --vault "LFX V2 - Development" --fields <field_name_1> --reveal) \
  --from-file=<field_name_2>=<(op item get "<1Password item name>" --vault "LFX V2 - Development" --fields <field_name_2> --reveal)

skills/lfx-v2-secrets/SKILL.md:300

  • This decision misses the case immediately described above: no chart-owned templates and customResources is absent/false. Proceeding directly to Step 3c then creates manifests that ArgoCD never applies. Require enabling customResources for each target environment before creating the static resources.
**If neither signal fires** → the absence is real; proceed to Step 3c as normal.

skills/lfx-v2-secrets/SKILL.md:551

  • The generic auth0_client_id/auth0_client_secret names are not unambiguous when one service has multiple Auth0 clients (for example, a BFF and M2M client). Both JSON objects receive the same ESO tag, and the configured conflictPolicy: Error then makes the entire merge fail. Require a client-specific prefix whenever more than one Auth0 credential is merged for a service.
> For LFX V2 services, always rename fields to a descriptive name prefixed with `auth0_`
> (e.g. `auth0_client_id`, `auth0_client_secret`) so keys are unambiguous in the merged K8s Secret.

skills/lfx-v2-secrets/SKILL.md:621

  • Provider tags such as atlassian or litellm can be shared by multiple services, so these examples do not ensure the workflow avoids deploying unrelated entries as claimed. Require a config tag whose uniqueness has been verified for the intended entry (or add a secret-specific tag) rather than presenting provider tags as safely scoped.
> 3. In the tag field, enter the most specific tag from the `tags:` field in your YAML entry
>    (e.g. `litellm`, `atlassian`) — not the AWS resource tag — to avoid re-deploying or
>    rotating unrelated secrets

skills/lfx-v2-secrets/SKILL.md:790

  • A SecretStore is consumed by the ESO controller; it is never mounted into the application pod. The final network check likewise needs to test the ESO controller's AWS reachability, not the pod's. As written, this sends debugging to two components that cannot establish whether the store is healthy.
1. **Pod events** — `kubectl describe pod <pod>` to see if the SecretStore mounted

skills/lfx-v2-secrets/SKILL.md:267

  • The inventory has only four items and never checks the per-environment IRSA annotations, yet Step 3d is governed by the rule that each Step 3 sub-step runs only when Step 2 found its item missing. If the four resources exist but a newly requested environment lacks its role annotation, line 266 skips Step 3 and that environment cannot authenticate. Include each requested environment's annotation as an independently checked item that gates Step 3d.
**Evaluate each of the four items independently** — the 2a IAM entry, the Helm
`serviceaccount.yaml`, `SecretStore.yaml`, and `ExternalSecret.yaml`:

skills/lfx-v2-secrets/SKILL.md:10

  • The bare "IRSA configuration" trigger overlaps the existing lfx-object-store-design and lfx-object-store-ops skills, which explicitly fire for IRSA/S3 work. Because this description says “whenever,” a non-secret IRSA request can load the wrong workflow. Scope the trigger to ESO/secrets and add the object-storage exclusion in the description, where skill selection can see it.
  objects exist before deciding which steps to run. Use this skill whenever someone says
  "set up secrets", "wire up ESO", "add a secret to this service", "IRSA configuration",
  "External Secrets for V2", or any mention of AWS Secrets Manager integration with
  Kubernetes for LFX V2 services.

@agaetep
agaetep requested a review from dealako August 7, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants