Improve setting up networking infra - #868
Open
lscalabrini01 wants to merge 13 commits into
Open
Conversation
lscalabrini01
force-pushed
the
qa-issue_2513-sg
branch
from
August 29, 2026 23:06
2d57b6b to
3ee18c2
Compare
lscalabrini01
marked this pull request as ready for review
August 31, 2026 13:30
lscalabrini01
requested review from
floatingman,
hamistao,
khushboo-rancher and
slickwarren
August 31, 2026 13:31
lscalabrini01
force-pushed
the
qa-issue_2513-sg
branch
from
August 31, 2026 18:19
fbe0ccc to
2bb3068
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
validation/pipeline/qainfra/Jenkinsfile.capi.e2e:114
- This log line prints env.RUNNER_PUBLIC_IP, but that environment variable is never set in this Jenkinsfile. This will typically log "null" and makes debugging harder.
echo "Detected runner public IP: ${env.RUNNER_PUBLIC_IP}"
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
hamistao
previously approved these changes
Sep 1, 2026
hamistao
left a comment
Contributor
There was a problem hiding this comment.
I just would like to know what do you think of rancher/qa-infra-automation#199 (comment) since it could impact this,
Comment on lines
+99
to
+111
| def publicIp = sh( | ||
| script: ''' | ||
| (curl -4 -fsS --max-time 5 https://ifconfig.me || | ||
| curl -4 -fsS --max-time 5 https://api.ipify.org || | ||
| curl -4 -fsS --max-time 5 https://ipinfo.io/ip) | tr -d '[:space:]' | ||
| ''', | ||
| returnStdout: true | ||
| ).trim() | ||
|
|
||
| def octets = publicIp.tokenize('.') | ||
| if (octets.size() != 4 || !octets.every { it ==~ /\d{1,3}/ && it.toInteger() >= 0 && it.toInteger() <= 255 }) { | ||
| error "Failed to auto-detect a valid public IPv4 address (got: '${publicIp}')" | ||
| } |
Contributor
There was a problem hiding this comment.
This being repeated makes me think this would be a good addition to qa-jenkins-library
slickwarren
previously approved these changes
Sep 2, 2026
lscalabrini01
dismissed stale reviews from slickwarren and hamistao
via
September 3, 2026 20:43
c15ca30
lscalabrini01
force-pushed
the
qa-issue_2513-sg
branch
2 times, most recently
from
September 4, 2026 21:21
b8eb302 to
7b1f8fe
Compare
floatingman
reviewed
Sep 4, 2026
Comment on lines
+573
to
+595
| stage('Detect Public IP') { | ||
| steps { | ||
| script { | ||
| def publicIp = sh( | ||
| script: ''' | ||
| (curl -4 -fsS --max-time 5 https://ifconfig.me || | ||
| curl -4 -fsS --max-time 5 https://api.ipify.org || | ||
| curl -4 -fsS --max-time 5 https://ipinfo.io/ip) | tr -d '[:space:]' | ||
| ''', | ||
| returnStdout: true | ||
| ).trim() | ||
|
|
||
| def octets = publicIp.tokenize('.') | ||
| if (octets.size() != 4 || !octets.every { it ==~ /\d{1,3}/ && it.toInteger() >= 0 && it.toInteger() <= 255 }) { | ||
| error "Failed to auto-detect a valid public IPv4 address (got: '${publicIp}')" | ||
| } | ||
|
|
||
| env.RUNNER_PUBLIC_IP = publicIp | ||
| echo "Detected runner public IP: ${env.RUNNER_PUBLIC_IP}" | ||
| } | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
If this will be used in multiple pipelines, it would be a good idea to move it to qa-jenkins-library.
| error "Failed to auto-detect a valid public IPv4 address (got: '${publicIp}')" | ||
| } | ||
|
|
||
| echo "Detected runner public IP: ${env.RUNNER_PUBLIC_IP}" |
Contributor
There was a problem hiding this comment.
I don't think env.RUNNER_PUBLIC_IP is ever assigned a value, so it's going to print null.
lscalabrini01
force-pushed
the
qa-issue_2513-sg
branch
from
September 4, 2026 23:34
7b1f8fe to
c6d0899
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves the security and flexibility of the Jenkins E2E infrastructure provisioning pipelines by dynamically detecting the runner's public IP address and using it to restrict SSH and load balancer ingress rules. It also cleans up and standardizes AWS networking configuration across several Jenkinsfiles, removing hardcoded security groups and making VPC, subnet, and security group assignments more explicit and consistent.
Key changes include:
Dynamic Public IP Detection and Usage
Jenkinsfile.e2e,Jenkinsfile.multibranch.e2e, and pipeline files undervalidation/pipeline/qainfra/). This IP is then injected into the pipeline environment and used in Terraform/Ansible configurations to restrict ephemeral security group ingress to only the runner's IP, enhancing security. [1] [2] [3] [4]Pipeline Environment and Configuration Updates
RUNNER_PUBLIC_IP, ensuring all downstream steps and configuration templates can use the correct IP. [1] [2] [3]Networking and Security Group Configuration Cleanup
Template and Config Variable Propagation
${RUNNER_PUBLIC_IP}placeholder in the configuration files. [1] [2] [3]These changes collectively improve security by minimizing SSH/LB exposure, increase maintainability, and reduce the risk of misconfiguration in CI/CD infrastructure automation.