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.

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

Consistently use sandbox for CpsFlowDefinitions #84

Closed
wants to merge 1 commit into from

Conversation

basil
Copy link
Member

@basil basil commented Jul 21, 2019

While looking at the test suite for this plugin, I noticed the CpsFlowDefinitions in the tests don't consistently use the script security sandbox. Using the script security sandbox results in a more realistic environment given that the script security sandbox should always be enabled in production.

In this change, I replaced any usages of the deprecated single-argument constructor for CpsFlowDefinition with usages of the non-deprecated two-argument constructor, passing in true as the second argument in order to always enable the script security sandbox.

Note that I did not touch ReplayActionTest#multibranch, which intentionally disables the script security sandbox for testing purposes.

Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

Thanks!

@dwnusbaum
Copy link
Member

@basil The GitHub UI kind of bugged out on me when I went to merge this, and now it says its conflicted. I'm not really sure what happened, but I don't think there should be any conflicts. I'll try to close and reopen to see if that does anything.

@dwnusbaum dwnusbaum closed this Dec 16, 2019
@dwnusbaum
Copy link
Member

Weird, it did get merged, see 488f87f, looks like some kind of concurrency issue.

basil pushed a commit to basil/workflow-multibranch-plugin that referenced this pull request Dec 16, 2019
Consistently use sandbox for CpsFlowDefinitions
@basil basil deleted the sandbox branch November 23, 2021 18:02
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