Skip to content

Strips newlines from AWS host/username secrets#842

Open
abdulazeem-tk4vr wants to merge 3 commits into
redhat-developer:mainfrom
abdulazeem-tk4vr:fix/tkn-sanitize-host-username-only
Open

Strips newlines from AWS host/username secrets#842
abdulazeem-tk4vr wants to merge 3 commits into
redhat-developer:mainfrom
abdulazeem-tk4vr:fix/tkn-sanitize-host-username-only

Conversation

@abdulazeem-tk4vr

@abdulazeem-tk4vr abdulazeem-tk4vr commented Jun 17, 2026

Copy link
Copy Markdown

Ensures that host and username values extracted from files for AWS host-access secrets are properly sanitized.

Previously, these values could include newline or carriage return characters, leading to incorrect base64 encoding and potential issues when creating Kubernetes Opaque secrets. Adding tr -d '\n\r' removes these characters, ensuring the secret data is clean and valid.

This PR helps close the issue in redhat-developer/mapt - #817

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: e18cc523-323d-4821-a43f-95509829c50b

📥 Commits

Reviewing files that changed from the base of the PR and between e638b0e and b297f39.

📒 Files selected for processing (9)
  • pkg/tkn/host_access_secret_test.go
  • tkn/infra-azure-fedora.yaml
  • tkn/infra-azure-rhel-ai.yaml
  • tkn/infra-azure-rhel.yaml
  • tkn/infra-azure-windows-desktop.yaml
  • tkn/template/infra-azure-fedora.yaml
  • tkn/template/infra-azure-rhel-ai.yaml
  • tkn/template/infra-azure-rhel.yaml
  • tkn/template/infra-azure-windows-desktop.yaml

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved generated connection-secret values by removing trailing newline/carriage-return characters before encoding, reducing formatting-related issues in host and username fields.
    • Applied the same cleanup to bastion-related fields and selected admin username fields where applicable.
  • Tests
    • Added coverage to verify secret encoding rules stay consistent across supported task templates.

Walkthrough

Tekton host-info secret generation now strips newline and carriage-return characters before base64 encoding selected fields across rendered and template YAML files. A new Go test scans infra task YAML files and checks that the expected sanitization rules are applied per field.

Changes

Host-info secret encoding

Layer / File(s) Summary
Secret encoding updates
tkn/infra-aws-fedora.yaml, tkn/infra-aws-mac.yaml, tkn/infra-aws-rhel-ai.yaml, tkn/infra-aws-rhel.yaml, tkn/infra-aws-windows-server.yaml, tkn/template/infra-aws-fedora.yaml, tkn/template/infra-aws-mac.yaml, tkn/template/infra-aws-rhel-ai.yaml, tkn/template/infra-aws-rhel.yaml, tkn/template/infra-aws-windows-server.yaml, tkn/infra-azure-fedora.yaml, tkn/infra-azure-rhel-ai.yaml, tkn/infra-azure-rhel.yaml, tkn/infra-azure-windows-desktop.yaml, tkn/template/infra-azure-fedora.yaml, tkn/template/infra-azure-rhel-ai.yaml, tkn/template/infra-azure-rhel.yaml, tkn/template/infra-azure-windows-desktop.yaml
Host-info secret generation now strips \n/\r before base64 -w0 for the affected host, username, and related airgap/admin fields, with field coverage varying by task file.
Encoding validation test
pkg/tkn/host_access_secret_test.go
A new test enumerates infra task YAML files, extracts host-info field references, and verifies which fields must include newline stripping and which must not.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is directly related to the main change, though it narrows the scope to AWS while the diff also touches Azure and tests.
Description check ✅ Passed The description is on-topic and accurately explains the newline-stripping fix for host and username secret values.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Tools execution failed with the following error:

Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@abdulazeem-tk4vr abdulazeem-tk4vr marked this pull request as ready for review June 17, 2026 22:18
This test ensures `tr -d '\n\r'` is correctly applied to host and username fields, but *not* to `id_rsa`, within AWS host-access secret templates. This prevents malformed private keys while ensuring host/username values are single-line.
@adrianriobo

adrianriobo commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

thx @abdulazeem-tk4vr for the contribution, can you check the failure it needs to catch the error I think, also can you extend the fix to all tekton tasks not only aws?

@adrianriobo

Copy link
Copy Markdown
Collaborator

@abdulazeem-tk4vr any chance you can take a look?

@abdulazeem-tk4vr

Copy link
Copy Markdown
Author

@abdulazeem-tk4vr any chance you can take a look?

sorry about the delay, I had lost access to some of my personal devices. I will quickly address this

Extend the newline stripping (`tr -d '\n\r'`) to `host`, `username`, and `adminusername` fields within Azure infrastructure task secrets. This ensures these string values are consistently single-line when base64-encoded, preventing potential parsing or usage issues in downstream systems.

The related test (`host_access_secret_test`) is generalized to validate this behavior for both AWS and Azure tasks. It now explicitly requires newline stripping for appropriate string fields while ensuring sensitive keys/passwords (e.g., `id_rsa`, `userpassword`) remain untouched to prevent corruption.
@abdulazeem-tk4vr

Copy link
Copy Markdown
Author

@adrianriobo all the checks have passed! Hopefully this is sufficient, if there are any further testing steps please let me know!

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.

2 participants