Skip to content
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

Jump to bottom

chore: turn off talisman for ephemeral environments in ci #24627

Merged
merged 1 commit into from
Jul 19, 2023
Merged

chore: turn off talisman for ephemeral environments in ci #24627

merged 1 commit into from
Jul 19, 2023

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Jul 7, 2023

SUMMARY

Talisman is throwing a lot of blocking script-src errors in the ephemeral testing environment for CI, and it's blocking login to the application in CI. I'm disabling Talisman since the CSP rules aren't needed there, unless for testing.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho
Copy link
Member Author

eschutho commented Jul 7, 2023

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

@eschutho Ephemeral environment spinning up at http://52.24.204.99:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho
Copy link
Member Author

eschutho commented Jul 8, 2023

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

@eschutho Ephemeral environment spinning up at http://34.217.88.36:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho
Copy link
Member Author

eschutho commented Jul 8, 2023

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

@eschutho Ephemeral environment spinning up at http://54.191.128.134:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

@eschutho Ephemeral environment spinning up at http://34.217.191.216:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho eschutho added the hold:testing! On hold for testing label Jul 12, 2023
@codecov
Copy link

codecov bot commented Jul 12, 2023 •
edited
Loading

Codecov Report

Merging #24627 (57ee90b) into master (2d58ddd) will increase coverage by 0.14%.
The diff coverage is n/a.

❗ Current head 57ee90b differs from pull request most recent head 77b7a5a. Consider uploading reports for the commit 77b7a5a to get more accurate results

@@            Coverage Diff             @@
##           master   #24627      +/-   ##
==========================================
+ Coverage   68.89%   69.03%   +0.14%     
==========================================
  Files        1901     1908       +7     
  Lines       73925    74210     +285     
  Branches     8183     8186       +3     
==========================================
+ Hits        50931    51232     +301     
+ Misses      20873    20857      -16     
  Partials     2121     2121
Flag Coverage Δ
hive 54.12% <ø> (-0.05%) ⬇️
mysql 79.42% <ø> (+0.20%) ⬆️
postgres 79.51% <ø> (+0.21%) ⬆️
presto 54.02% <ø> (-0.05%) ⬇️
python 83.49% <ø> (+0.17%) ⬆️
sqlite 78.07% <ø> (+0.18%) ⬆️
unit 54.83% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/config.py 92.17% <ø> (ø)

... and 46 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@eschutho
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

@eschutho Ephemeral environment spinning up at http://35.86.154.205:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

@eschutho Ephemeral environment spinning up at http://35.165.13.75:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@eschutho eschutho force-pushed the elizabeth/fix-ephemeral branch 2 times, most recently from c2d4565 to 7621742 Compare July 12, 2023 20:13
@eschutho
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

@eschutho Ephemeral environment spinning up at http://54.190.184.191:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

eschutho
eschutho commented Jul 12, 2023
View reviewed changes
superset/config.py Outdated
@@ -1397,6 +1397,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
},
"content_security_policy_nonce_in": ["script-src"],
"force_https": False,
"session_cookie_samesite": "None",
Copy link
Member Author

Choose a reason for hiding this comment

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

just testing if this works.. the long term solution would be to put this in the github workflow above.

@eschutho eschutho removed the hold:testing! On hold for testing label Jul 18, 2023
@eschutho
Copy link
Member Author

@michael-s-molina we had talked about solving config issues for CSP so that we can keep it on in ephemerals. Since we've been blocked on using ephemerals for a bit now, I'm leaning towards merging this to turn off CSP and then coming back shortly afterward to test turning it back on since it could take longer to solve. WDYT?

@michael-s-molina
Copy link
Member

@michael-s-molina we had talked about solving config issues for CSP so that we can keep it on in ephemerals. Since we've been blocked on using ephemerals for a bit now, I'm leaning towards merging this to turn off CSP and then coming back shortly afterward to test turning it back on since it could take longer to solve. WDYT?

Sounds good 👍🏼

@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@michael-s-molina
Copy link
Member

@eschutho Could you rebase this PR to fix the Docker issue?

@eschutho eschutho merged commit 1a97245 into apache:master Jul 19, 2023
26 checks passed
@eschutho eschutho deleted the elizabeth/fix-ephemeral branch July 19, 2023 21:22
@michael-s-molina michael-s-molina added the v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch label Jul 20, 2023
michael-s-molina pushed a commit that referenced this pull request Jul 26, 2023
@eschutho @michael-s-molina
chore: turn off talisman for ephemeral environments in ci (#24627)
1625fe4 
(cherry picked from commit 1a97245)
@eschutho eschutho mentioned this pull request Aug 4, 2023
Merged
9 tasks
@mistercrunch mistercrunch added 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@craig-rueda craig-rueda craig-rueda approved these changes

@villebro villebro Awaiting requested review from villebro villebro is a code owner

@geido geido Awaiting requested review from geido geido is a code owner

@rusackas rusackas Awaiting requested review from rusackas rusackas is a code owner

@betodealmeida betodealmeida Awaiting requested review from betodealmeida betodealmeida is a code owner

@nytai nytai Awaiting requested review from nytai nytai is a code owner

@mistercrunch mistercrunch Awaiting requested review from mistercrunch mistercrunch is a code owner

@john-bodley john-bodley Awaiting requested review from john-bodley john-bodley is a code owner

@kgabryje kgabryje Awaiting requested review from kgabryje kgabryje is a code owner

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch 🍒 3.0.0 🍒 3.0.1 🍒 3.0.2 🍒 3.0.3 🍒 3.0.4 🚢 3.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eschutho @michael-s-molina @craig-rueda @mistercrunch