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

Improve migration docs #220

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Improve migration docs #220

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aff1fb6
Describe remaining migration work
Peter-Darton-i2 Sep 15, 2023
bfaa8a8
Don't dev-publish for forked-PRs. Only pack if tests worked
Peter-Darton-i2 Sep 15, 2023
09be881
Example should only dev-publish for forked-PRs and only pack if tests…
Peter-Darton-i2 Sep 15, 2023
549aa4c
Moved changes to test-deploy.yml and example to new PRs
Peter-Darton-i2 Jan 12, 2024
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
16 changes: 10 additions & 6 deletions .circleci/test-deploy.yml
100755 → 100644
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove all changes to the CI process to keep this a docs only change. Was there a reason for these config changes?

Copy link
Author

Choose a reason for hiding this comment

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

The reasons are two-fold.

Improved example of how to build an orb

This orb is the orb that builds orbs; it is "the primary source of truth" when it comes to building orbs.
As a result, the documentation here (especially the example code) in this orb contains the most "official" recommendation for how to build an orb.

This PR updates the example orb-build code such that CI builds from forked-PRs don't try to do a dev-publish (as they wouldn't have access to the context, e.g. as demonstrated by the failure in this orb PR).
i.e. this makes the orb build process suitable for use "in the more general usecase".

This change is an essential step on the road towards enabling forked-PR builds on CircleCI's public orbs.
While I accept that this step isn't the only thing that'll need to change (as per your previous comments about malicious PRs), it is the only thing that will need to change within this orb's source code - all the other concerns will be solved by changes in other places.

i.e. this change to "how we should all be building orbs" is "the final link in the chain" towards having viable public CI builds of forked-PRs on orb code.

Dogfooding

aka "Eating your own dog food" aka "Drinking our own champagne".
If we're going to tell everyone else to do this, we should "Put our money where our mouth is" and do it ourselves.

... and because this orb is, itself, an orb, it should follow its own recommendations (otherwise we'll look insincere and hypocritical).

In summary

We should all be doing orb builds in a forked-PR-friendly manner, including CircleCI's orb-tools orb.

Copy link
Contributor

Choose a reason for hiding this comment

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

While I agree with the changes shown here should be added, we can not accept them as part of the same PR that makes changes to the docs, as we are going to be required to squash the commits and rename them using our conventional commit standard.

I recommend we break this up into at least two separate PRs, one for the migration guide, and one for the CI and usage example changes.

Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,29 @@ workflows:
jobs:
- orb-tools-alpha/lint:
filters: *filters
- orb-tools-alpha/pack:
filters: *filters
- orb-tools-alpha/review:
filters: *filters
- orb-tools-alpha/pack:
filters: *filters
requires:
- orb-tools-alpha/lint
- orb-tools-alpha/review
- orb-tools-alpha/publish:
name: publish_dev_test
orb_name: circleci/orb-tools
vcs_type: <<pipeline.project.type>>
pub_type: dev
github_token: GHI_TOKEN
requires:
[orb-tools-alpha/lint, orb-tools-alpha/review, orb-tools-alpha/pack]
requires: [orb-tools-alpha/pack]
context: orb-publisher
filters:
branches:
ignore: /pull/[0-9]+/
- orb-tools-alpha/publish:
orb_name: circleci/orb-tools
vcs_type: <<pipeline.project.type>>
pub_type: production
requires:
[orb-tools-alpha/lint, orb-tools-alpha/review, orb-tools-alpha/pack, publish_dev_test]
requires: [orb-tools-alpha/pack]
context: orb-publisher
github_token: GHI_TOKEN
filters:
Expand Down
33 changes: 32 additions & 1 deletion MIGRATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ _Note: If you are upgrading from 11.x to 12.x proceed here. If you are upgrading

- **Removed the need for a dev publishing token**
- Previously, after the orb was built, it would be published to a development tag and referenced in the test pipeline. This required a publishing token to be present in the dev pipeline.
- The dynamic configuration system allows us to inject the orb directly into the pipeline as an "in-line" orb, allowing for orb testing without the need for access to a publishing token.
- The dynamic configuration system allows us to inject the orb directly into the pipeline as an "in-line" orb, allowing for orb testing without the need for access to a publishing token,
thus allowing forked-PR builds to "build and test" securely.
- **Snake_Case Renamed Components**
- To keep consistency with CircleCI's _native_ configuration schema, all components and parameters have been renamed to use snake_case.
(Example: `parameter-name` -> `parameter_name`)
Expand All @@ -21,16 +22,46 @@ _Note: If you are upgrading from 11.x to 12.x proceed here. If you are upgrading

1. Clone your orb project locally.
1. Create a new branch for upgrading your pipeline

```sh
git checkout -b orb-tools-12-migration
````

1. Copy the `migrate.sh` script from this repository into your project's root directory.
1. Run the script

```sh
chmod +x migrate.sh && ./migrate.sh
```

After executing the script, your orb's component names and parameters will be converted to snake_case, and references to them within your config files will be updated. The reference to your orb in the `test-deploy` config will be removed, as the orb will now be dynamically injected into the pipeline.

1. Edit your project's `.circleci/config.yml` file.

Compare the current
[v12 "step1 " lint-pack example](https://circleci.com/developer/orbs/orb/circleci/orb-tools?version=12#usage-step1_lint-pack)
vs the
[v11 version](https://circleci.com/developer/orbs/orb/circleci/orb-tools?version=11#usage-step1_lint-pack)
to see the required changes.
- Remove the old "dev publish" job (`orb-tools/publish`) from the workflow.
- Fix the `requires:` specification so that the "continue" job requires everything else.

1. Edit your project's `.circleci/test-deploy.yml` file.

Compare the current
[v12 "step2 " test-deploy example](https://circleci.com/developer/orbs/orb/circleci/orb-tools?version=12#usage-step2_test-deploy)
vs the
[v11 version](https://circleci.com/developer/orbs/orb/circleci/orb-tools?version=11#usage-step2_test-deploy)
to see the required changes.

- Add an extra call to the "pack" job (`orb-tools/pack`) and ensure that the publish job(s) `require:` it.
- Consider adding an extra call to the "publish" job to reinstate the "dev publish" removed from the `config.yml` build file.
- Not all users will require this; this is only necessary if dev publishes of unreleased orbs are needed for use by other projects.
- Consider setting a filter to ensure that the dev publish doesn't happen for release builds (i.e. no tags) or for external (forked) PR builds (i.e. no branches matching `/^pull/[0-9]+$/`).
- Make the "pack" job `require:` all your test jobs and then have the publish job(s) `require:` the pack job.

---

## v11.0.0

Version 11.0.0 of orb-tools is composed of a major re-write that greatly changes and improves the way orbs are deployed, makes use of CircleCI's [dynamic configuration](https://circleci.com/docs/2.0/dynamic-config/), and can even automatically test for best practices.
Expand Down
23 changes: 20 additions & 3 deletions src/examples/step2_test-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,30 @@ usage:
tags:
only: /.*/
# Because our publishing job has a tag filter, we must also apply a filter to each job it depends on.
- orb-tools/pack:
requires:
- command-tests
- <my-orb>/my_job
filters:
tags:
only: /.*/
- orb-tools/publish:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure we need to bake in the dev orb publishing process by default. It may be good for documentation but I'm worried it could also complicate the example as it is not a required step.

Copy link
Author

Choose a reason for hiding this comment

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

I guess it depends what you mean by "required".
I mean, strictly speaking, folks could code an orb by doing cat | curl -X POST ... and then typing really carefully. It wouldn't be an efficient coding method ... but it is possible.
Similarly, folks don't need to run shellcheck, bats etc; it's not "required" but it is very much recommended.
A "normal" and productive orb development method is to commit to a PR branch and make use of the dev-published version of the orb it generates.
e.g. Most of the time I'm writing orb code, it's for a specific usecase where I've got a build (somewhere else) that needs "the thing I'm writing in the orb", and so after I've done a dev orb publish, I'll go tell my other build to use that dev orb "to prove it works in the real world".
It's incredibly rare that I'll go straight from "build was green" to "release an official version", so it's very rare that I won't want a dev publish, and if I really didn't want one, I could always comment out that bit of the build in my PR. Commenting out code is a lot easier than adding it.

IMO, if we're only going to have one single usage example, that example ought to be "an example of best practice" ... and IMO most users will want dev publishes (not on forked PRs of course, but on internal ones).

So, while I agree that it does make things a little more complicated, if we remove it then either "just about everyone" will need to manually re-add it or they'll end up developing orbs without access to this almost indispensable feature and find that they're making their development experience needlessly hard.

TL;DR: I understand your concerns ... but I disagree 😉

name: publish_dev_test
orb_name: <namespace>/<my-orb>
pub_type: dev
vcs_type: <<pipeline.project.type>>
requires: [orb-tools/pack]
context: [orb-publishing-context]
filters:
tags:
ignore: /^v[0-9]+\.[0-9]+\.[0-9]+$/
branches:
ignore: /^pull/[0-9]+/
- orb-tools/publish:
orb_name: <namespace>/<my-orb>
pub_type: production
vcs_type: <<pipeline.project.type>>
requires:
- command-tests
- <my-orb>/my_job
requires: [orb-tools/pack]
context: [orb-publishing-context]
filters:
tags:
Expand Down