Skip to content

Merge dev changes into mainline - #225

Merged
muskanlalit18 merged 22 commits into
mainlinefrom
dev-golang-2.0
Jun 1, 2026
Merged

Merge dev changes into mainline#225
muskanlalit18 merged 22 commits into
mainlinefrom
dev-golang-2.0

Conversation

@muskanlalit18

Copy link
Copy Markdown
Collaborator

This PR merges the latest changes from dev into mainline.

spotaws and others added 22 commits March 19, 2026 11:53
Fedora has specific packaging standards for Go applications. The files in
fedora/ are in the format that Fedora requires. The go2rpm tooling generates
a second tarball containing all Go module dependencies for air-gapped builds.

Cherry-picked from https://github.com/spotaws/credentials-fetcher commit 2a11d82

cr: https://code.amazon.com/reviews/CR-261424953
…letion

- Prevent cleanup errors from shadowing original API errors by using
  separate cleanupErr variable instead of redeclaring err with :=
- Log cleanup failures instead of returning them to surface the real error
- Switch CleanupKerberosFilesWithFS from Remove to RemoveAll for recursive
  directory deletion on failure
- Wrap GetDistinguishedName error with service account context
- Update tests to reflect RemoveAll usage and new error messages

feat: add debug-only simulated error injection for API error handling

Add a mechanism to simulate errors during API processing that only
activates when LOG_LEVEL=debug and CF_DEBUG_SIMULATE_ERROR=<operation>.
Stops simulating after 3 invocations to prevent infinite error loops.

Supported operations:
  - setup_kerberos_file: simulates SetupKerberosFileForTicket failure
  - create_ticket_gmsa: simulates CreateTicketForGMSA failure
  - get_distinguished_name: simulates GetDistinguishedName failure

Example:
  LOG_LEVEL=debug CF_DEBUG_SIMULATE_ERROR=create_ticket_gmsa ./credentials-fetcher

This allows verifying that the correct original error is surfaced and
cleanup runs properly without requiring real AD/Kerberos infrastructure
failures. Safe for production since LOG_LEVEL defaults to info and
CF_DEBUG_SIMULATE_ERROR is never set in normal operation.

cr: https://code.amazon.com/reviews/CR-263941434
- Support blue/green username rotation in NDJ Kerberos lease renewal
  via "oldUser:newUser" format for seamless credential rotation
- Clarify that password rotates alongside username in docs
- Make distinguished name lookup non-fatal with error propagation
  when both ECS config and secrets manager fail
- Propagate distinguished name through ARN lease credential extraction
- Move debug error injection to internal/utils/debug_utils
- Add LICENSE.txt and update spec files

cr: https://code.amazon.com/reviews/CR-265449900
- Remove '.' from invalid character blocklist — period is valid in AD
  sAMAccountName per Microsoft spec
- Strip UTF-8 BOM (EF BB BF) before JSON parsing of credential spec files
- Align blocklists with official AD spec: add missing chars (", =, ,, (, ), {, })
- Consolidate sAMAccountName validation into single source of truth
  (constants.InvalidUsernameChars)
- Split credential spec path validation into separate InvalidCredSpecPathChars
  (excludes '/' since mount paths contain forward slashes)
- Add tests for BOM handling, new invalid chars, and valid period usernames

References:
- https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/bb726984(v=technet.10)
- https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou

SIM: https://i.amazon.com/P408666926
cr: https://code.amazon.com/reviews/CR-266244370
Bumps [github.com/aws/aws-sdk-go-v2/service/s3](https://github.com/aws/aws-sdk-go-v2) from 1.53.0 to 1.97.3.
- [Release notes](https://github.com/aws/aws-sdk-go-v2/releases)
- [Commits](aws/aws-sdk-go-v2@service/s3/v1.53.0...service/s3/v1.97.3)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go-v2/service/s3
  dependency-version: 1.97.3
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

cr: https://code.amazon.com/reviews/CR-266512807
…rotocol/eventstream

Bump google.golang.org/grpc from 1.72.0 to 1.79.3
Bump github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream from 1.6.1 to 1.7.8

cr: https://code.amazon.com/reviews/CR-266534886
- Strip lone 0xEF byte prefix from partial/corrupt BOM sequences in parseJSON
- Log first 32 bytes as hex on JSON parse failure for diagnostics
- Add unit tests for lone 0xEF, partial BOM, and invalid JSON hex logging

cr: https://code.amazon.com/reviews/CR-268192502
Fix two bugs in the GMSA ticket renewal flow that caused a ~10 minute
window where the GMSA ticket was expired and unusable:

1. After the domainless user ticket was successfully recreated,
   recreateTicketWithRetries returned immediately instead of retrying
   GMSA ticket creation with the now-valid user credentials. Changed
   return nil to continue so the retry loop proceeds to the next
   iteration. This bug existed in the original C++ implementation
   (renewal.cpp) and was carried over to the Go rewrite.

2. After RenewKerberosTicket via CGO reported success, the code did
   not verify the ticket expiry was actually extended. When the KDC
   policy sets renewable lifetime equal to ticket lifetime, renewal
   succeeds but the expiry does not move forward. Added a post-renewal
   verification that re-reads the ticket and confirms the expiry was
   extended, falling through to full ticket recreation if not.

cr: https://code.amazon.com/reviews/CR-269278074
…Lease

Parse "oldUser:newUser" colon-separated format before validation so that
the active (green) username passes ValidateAccountName. Previously the
raw string was rejected because ":" is in InvalidSAMAccountNameChars.

- Add ParseBlueGreenUsername call at top of AddNonDomainJoinedKerberosLease
- Use activeUsername for validation, ProcessCredentialSpecs, and
  CreateKerberosTickets so metadata stores only the resolved username
- Return clear error when new username side is empty ("oldUser:")
- Add unit tests and full gRPC API tests covering the blue/green format
  for both Add and Renew flows

cr: https://code.amazon.com/reviews/CR-271208861
…DomainlessUser is set

In ECS mode, RenewNonDomainJoinedKerberosLease incorrectly fell back to
reading the secret from Secrets Manager using the instance role when
DomainlessUser didn't match the request username. The instance role
doesn't have secretsmanager:GetSecretValue in ECS deployments, causing
AccessDeniedException.

The fix skips the Secrets Manager fallback when DomainlessUser is already
populated in metadata — a non-match simply means the ticket isn't for
this user. The fallback is only needed in open-source/standalone mode
where DomainlessUser may be empty.

Also adds test documentation for blue-green username rotation
and unit tests for the fallback skip logic.

cr: https://code.amazon.com/reviews/CR-273723064
When the secret has oldUser:newUser format and the rotation has already
been applied (metadata shows newUser), subsequent renewals failed with
'No matching tickets found' because only oldUser was checked.

Now the renew flow also matches tickets by the active (new) username
when the old username isn't found. If tickets already have the active
username, they are renewed normally via direct kinit instead of being
recreated.

cr: https://code.amazon.com/reviews/CR-274331435
The Go rewrite was missing log rotation, causing unbounded log file
growth (~10-14 MB/hour under active renewal). This matches the C++
version (v1.3.8) behavior which truncates the log file when it exceeds
10 MB.

Rotation is checked at startup and every 5 minutes via a background
goroutine. No external dependencies added.

cr: https://code.amazon.com/reviews/CR-274376751
The %post script was missing 'systemctl enable', so credentials-fetcher
would not start automatically after a system reboot. This caused tasks
to fail after scheduled OS patching reboots.

cr: https://code.amazon.com/reviews/CR-274522547
After instance reboots, the ECS agent loses track of previously running
tasks and their Kerberos tickets remain on disk indefinitely. Over time
with scheduled patching reboots, these orphaned tickets accumulate.

Added CleanupOrphanedTickets which runs hourly and removes lease
directories where all tickets have been expired for more than 7 days
past their renew_until time.

Also addresses Heimdall edge-case concerns from CR-274376751:
- Silent truncation failures: log to stderr when os.Truncate fails
- Double-close panic: use sync.Once to guard channel close
- Race condition: acceptable copytruncate semantics (documented)

cr: https://code.amazon.com/reviews/CR-274540361
…newals

Problem:
When credentials-fetcher is running on Fargate, the ECS agent manages
ticket renewals via RenewKerberosArnLease gRPC calls. The username did
not have the awsdomainlessusersecret prefix, causing the renewal to be
skipped silently — tickets expired without being renewed.

Additionally, the logger was writing to stdout which causes a pipe buffer
deadlock when running in detached containers (ctr run -d).

Fix:
- AddKerberosArnLease: Set DomainlessUser to 'awsdomainlessusersecret:'
  + secretsArn after ticket creation (kinit uses real username, metadata
  stores ARN for renewal identification)
- Skip internal renewal for agent-managed tickets (check moved before
  GetTicket to avoid unnecessary klist subprocess)
- Logger: write to stderr + log file (not stdout)

Reference: Matches AddKerberosLease behavior on EC2 with C++ CF v1.3.8
which sets DomainlessUser to 'awsdomainlessusersecret:<secretName>'.

cr: https://code.amazon.com/reviews/CR-275497167
The "Blue/green username rotation detected" INFO log was emitted every
renewal cycle whenever the username contained a colon separator, even
when no rotation was actually needed. This caused confusion for
customers reviewing logs after a one-time rotation event.

Move the log to fire only when tickets genuinely need rotating
(needsRotation=true), and replace the early log with a neutral message
noting the colon separator format.

cr: https://code.amazon.com/reviews/CR-277587666
- docs/userdata.md: Fix tilde formatting (~ecs.service~ -> ecs.service)
- opensource/Makefile: Fix build output path (remove opensource/ prefix)
- OPENSOURCE_README.md: Update binary name to credentials-fetcher, fix
  PluginInput credspec format to use CredentialArn object
@muskanlalit18
muskanlalit18 merged commit 61bf2a1 into mainline Jun 1, 2026
3 checks passed
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.

3 participants