fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jam-jee wants to merge 1 commit into from
Closed

fix(ci): guard fast-integ-tests to same-repo PRs #6193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions .github/workflows/pr-checks-master.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
fast-integ-tests:
runs-on: ubuntu-latest
needs: [detect-changes]
if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
# Same-repo PRs only, because this is the one job here that puts PR code on a
# runner. actions/checkout refuses to place a fork's head commit in a
# pull_request_target job -- correctly: this job holds the base repo's
# GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
# and read those credentials out. That is the "pwn request" shape, and on a
# public repo it is a live credential-exfiltration path, so the guard skips
# the job rather than overriding the refusal with allow-unsafe-pr-checkout.
#
# Known consequence: nearly every PR to this repo comes from a fork, so this
# is not yet a gate on the PRs that matter. Closing that gap means running
# the suite where the other three test jobs already run PR code -- inside
# CodeBuild via source-version-override, which never exposes the runner's
# token or secrets to it (see integ-tests above). That needs a
# <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
# existing ones. Until it exists this suite is effectively a local signal --
# whether the daily CI-health run over refs/heads/master reaches
# tests/integ/train/shallow depends on that buildspec's selection, which also
# lives outside this repo.
if: >-
contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
&& github.event.pull_request.head.repo.full_name == github.repository
steps:
- uses: actions/checkout@v3
with:
# pull_request_target checks out the base ref by default; these tests
# must run against the PR's code.
# must run against the PR's code. Safe only because the job is guarded
# to same-repo PRs above -- a branch in this repo already requires push
# access, so checking it out grants nothing its author lacks.
ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

- name: Set up Python
Expand Down
15 changes: 15 additions & 0 deletions sagemaker-train/tests/integ/train/shallow/README.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(ci): guard fast-integ-tests to same-repo PRs#6193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Closed

jam-jee
wants to merge
1
commit into

from

 Closed


fix(ci): guard fast-integ-tests to same-repo PRs
#6193
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


1 commit


Select commit


File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


26 changes: 24 additions & 2 deletions
.github/workflows/pr-checks-master.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -272,12 +272,34 @@ jobs:
 fast-integ-tests:
 runs-on: ubuntu-latest
 needs: [detect-changes]
 if: contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 # Same-repo PRs only, because this is the one job here that puts PR code on a
 # runner. actions/checkout refuses to place a fork's head commit in a
 # pull_request_target job -- correctly: this job holds the base repo's
 # GITHUB_TOKEN and assumes CI_AWS_ROLE_ARN, so a fork could edit conftest.py
 # and read those credentials out. That is the "pwn request" shape, and on a
 # public repo it is a live credential-exfiltration path, so the guard skips
 # the job rather than overriding the refusal with allow-unsafe-pr-checkout.
 #
 # Known consequence: nearly every PR to this repo comes from a fork, so this
 # is not yet a gate on the PRs that matter. Closing that gap means running
 # the suite where the other three test jobs already run PR code -- inside
 # CodeBuild via source-version-override, which never exposes the runner's
 # token or secrets to it (see integ-tests above). That needs a
 # <repo>-ci-sagemaker-train-fast-integ-tests project in the CDK that owns the
 # existing ones. Until it exists this suite is effectively a local signal --
 # whether the daily CI-health run over refs/heads/master reaches
 # tests/integ/train/shallow depends on that buildspec's selection, which also
 # lives outside this repo.
 if: >-
 contains(fromJson(needs.detect-changes.outputs.submodules), 'sagemaker-train')
 && github.event.pull_request.head.repo.full_name == github.repository
 steps:
 - uses: actions/checkout@v3
 with:
 # pull_request_target checks out the base ref by default; these tests
 # must run against the PR's code.
 # must run against the PR's code. Safe only because the job is guarded
 # to same-repo PRs above -- a branch in this repo already requires push
 # access, so checking it out grants nothing its author lacks.
 ref: 'refs/pull/${{ github.event.pull_request.number }}/head'

 - name: Set up Python
Expand Down

15 changes: 15 additions & 0 deletions
sagemaker-train/tests/integ/train/shallow/README.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,21 @@ What this suite changes about the gate is not which job runs, but what the exist
one selects: the `gpu_intensive` marks added here deselect the deep tests that
submit a job and wait for it, and this suite covers those code paths instead.

> **Current limitation — the job only runs on same-repo PRs.** `fast-integ-tests`
> is the one job in `pr-checks-master.yml` that puts PR code on a GitHub runner,
> and that runner assumes `CI_AWS_ROLE_ARN`. `actions/checkout` refuses to place a
> fork's head commit in a `pull_request_target` job for exactly that reason, so the
> job is guarded to same-repo PRs. Almost every PR here comes from a fork, so in
> practice this is presently a **local** signal rather than a gate. The fix is to
> run it the way the other three test jobs run PR code — inside CodeBuild via
> `source-version-override`, which never exposes the runner's token or secrets to
> it — which needs a CodeBuild project provisioned alongside the existing ones.
>
> Until then, run this suite locally when changing a submission path, and note that
> the deselection described above trades gate coverage for coverage that depends on
> whether the daily CI-health buildspec reaches this directory — that selection
> lives outside this repo and is worth confirming.

## What a passing test proves

Each test submits a real `CreateTrainingJob`, asserts the service returned a
Expand Down
Loading