search

CircleCI-Public / orb-tools-orb

Various tools for authoring and publishing CircleCI orbs
https://circleci.com/orbs/registry/orb/circleci/orb-tools
MIT License
50 stars 74 forks source link

Improve migration docs #220

Open Peter-Darton-i2 opened 11 months ago

Peter-Darton-i2 commented 11 months ago

The existing v11 -> v12 migration documentation is missing some necessary information. The net effect is that, if one follows the documented migration process as-is, it doesn't work.

This PR, together with #229 and #230, fixes that. ... and also makes this orb's test-deploy (step2) build "practise what it preaches" by avoiding the dev-publish step for forked-PR builds (as they should fail to get a context).

KyleTryon commented 10 months ago

A little late here but if you are willing to rebase, we have got the examples update so we can safely remove them from this PR. Thanks for the help!

Peter-Darton-i2 commented 9 months ago

I've rebased the PR as requested.

Note: You can safely enable the CircleCI -> Project settings -> Advanced "Build forked PRs" flag for this PR ... and once this is merged, you can safely leave it enabled because this PR turns off the dev-publish job (which requires a context) for forked PRs. i.e. once this is "in" you can let CircleCI do the builds that GitHub has been told to insist upon.

(and if you update other orbs' test-deploy.yml files to follow this pattern, you can then enable forked-PR builds for those too, which will really help non-CircleCI folks contribute usefully to the orb code and generally make your job a little easier 😉)

PS. ensure that the "Pass secrets to builds from forked pull requests" flag is OFF (it defaults to "on"). That setting should never be "on" for a publicly-visible repo.

KyleTryon commented 9 months ago

Highly appreciate the help. Unfortunately it was not (only) the publish job that forces us to disable forked builds. We may still experiment with turning it back on, but the original reason was due to crypto miners. We used to get a LOT of spam PRs that totally changed the config to mine crypto. We actually have a config policy feature that I haven't yet to dove too deeply into which can probably help too.

Peter-Darton-i2 commented 9 months ago

Crypto miners? Wow - that's "unsociable" ... but I guess it's not entirely unexpected; any loophole will be exploited if it can be.

I've been told that OPA config policies are "the CircleCI answer" to all of those security concerns ... although I've personally found that OPA syntax rather difficult; if you do find an answer then please do share because we want to have some of our own CircleCI builds be public and forkable too...

Peter-Darton-i2 commented 6 months ago

OK, understood. I'll split things up ... although y'all have to merge "both together" or "neither" in order to avoid inconsistencies. As is, this PR keeps everything nice and consistent, ensuring that the orb itself builds using code that's really similar to the example code.

(Sorry for the delay in responding; had an end-of-year work-rush, followed by xmas break. I'll get on with this as soon as I've cleared the holiday backlog)

Peter-Darton-i2 commented 6 months ago

I've split this PR up. See #229 and #230.