Skip to content

Conversation

@sealesj
Copy link

@sealesj sealesj commented Sep 26, 2023

Initial testing configuration for autopublishing to use Google Cloud Build.

This change requires the pub.dev publishing to migrate into a Google Cloud Build configuration file which is triggered by a flutter GCP trigger. The tagging in the release.yml will continue to execute, and the tagging push will trigger the GCB yaml.

In this state, the current autopublishing flow would still be enabled, but we could move forward with providing access to the flutter-infra GCP project to impersonate a service account for releasing

Addresses: flutter/flutter#126827

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@sealesj sealesj marked this pull request as draft September 27, 2023 18:44
@sealesj sealesj marked this pull request as ready for review September 27, 2023 20:54
@sealesj sealesj requested a review from godofredoc September 28, 2023 16:24
@sealesj
Copy link
Author

sealesj commented Sep 28, 2023

@godofredoc would flutter-infra be an appropriate project to re-use for configuring a service account to use this yaml?

@godofredoc
Copy link
Contributor

@godofredoc would flutter-infra be an appropriate project to re-use for configuring a service account to use this yaml?

No, we need to create a new project for this.

@sealesj
Copy link
Author

sealesj commented Sep 28, 2023

No, we need to create a new project for this.

Ok, the changes should be pretty straightforward on the project, but I don't have the necessary permissions to make a new GCP project in the flutter parent organization

I'll make a new GCP project and see how far I can get with configuring the service account on that

cloudbuild.yml Outdated
# TODO @jseales - replace --dry-run on dart command with --force
script: |
cat /secrets/temporary-pub-token.txt | dart pub token add https://pub.dev
dart pub publish --dry-run
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you describe the chain of events involved for this flow, in particular where this script is run (not our CI I assume?) and how the environment on that machine is configured? What would cause this to be in the correct directory?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for those questions. I was using this starting cloudbuild configuration to test the connection with the existing github action. My thought is that the existing github action will continue executing all the steps up to the dart publish, and that the tag push will trigger the GCB yaml.

From then I am thinking in the new GCB yaml configuring a new service account with permission to publish to pub which is authenticated in the yaml would allow pub publishing.

This yaml would only need the gcloud and dart cloud builders to run on google cloud.
However, I am not sure for the directory question and need to continue testing

Copy link
Author

Choose a reason for hiding this comment

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

I've tested using a new GCP project. The connection is working, and it seems like the pub publish command in the tool workspace is working as well. The remaining step would be to now allow publishing on the pub side for each package.

The dry-run seems to be working as expected, also showing failures since these release were not tagged which is expected, and this workflow would trigger only after the tag has already been pushed by the existing release.yml workflow.
Screenshot 2023-10-12 at 3 56 53 PM

From documentation I see that invoking this tool to publish is deprecated. Would using this tool to execute pub publish be acceptable here, or would you prefer a refactoring of the publishing so that it is invoked in a different way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that you have it wired up, could you walk me through the flow (i.e., my comment above)? Once we tag a plugin, what is the exact chain of events? Currently I know how everything works in our release system (e.g., we are directly controlling the version of Dart being used, our tooling is controlling the checkout, directory, etc. management), and I'd like to have an understanding (and ideally documentation in the wiki) of exactly what is happening in the new system, both so that I can reason about its correctness (it's important that the checkout have the hash corresponding to the tag, for instance; what manages that?) and so that we know what to do if something goes wrong since it will no longer be directly under repo control.

Copy link
Collaborator

Choose a reason for hiding this comment

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

From documentation I see that invoking this tool to publish is deprecated. Would using this tool to execute pub publish be acceptable here, or would you prefer a refactoring of the publishing so that it is invoked in a different way?

In this context using pub publish directly should be fine, since our tooling will be controlling the tagging process.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could using pub-publish-flags=--dry-run with the current publish be a solution using existing tooling?

It would work, but it's pretty wasteful since we already run publish --dry-run as part of our standard test suite, so this would pointlessly duplicate work in CI.

Why would publish without tagging be necessary?

See #5005 (comment)

Copy link
Author

@sealesj sealesj Nov 2, 2023

Choose a reason for hiding this comment

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

@stuartmorgan Thank you for such great conversation through this issue! Could I help contribute to the necessary repo tooling changes?

From the GCP side, the configuration is good to go with the exception of security hardening after the workflow is set up. Those changes are:

  • requiring approval before a build executes if it is triggered manually
  • disable manual ability to trigger with only exception for something like members of an access on demand group

And there is also the pending task of allowing the GCP service account to be a publisher to each of the pub.dev packages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could I help contribute to the necessary repo tooling changes?

Sure; they are on my to-do list, but if you have cycles to get to any of it before I do that would be great, and I'm happy to review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So to summarize, I think the repo tooling changes we need are:

  • a flag for publish to tag without publishing
  • a flag for publish to publish without tagging (maybe validating that the tag exists already, and for the current hash)
  • a flag to disable group-name support for --package

#6218 will add all of these.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for this update!

@stuartmorgan-g
Copy link
Collaborator

Update from triage: this is blocked on repo tooling work. I hope to spend some time on that this month.

auto-submit bot pushed a commit that referenced this pull request Mar 5, 2024
Adds the flowing to the tool:
- A new `--exact-match-only` flag to be used with `--packages` to prevent group matching (i.e., a selection like `--packages=path_provider --exact-match-only` would only run on `packages/path_provider/path_provider`, not `packages/path_provider/*`).
- Two new `publish` command flags:
  - `--tag-for-auto-publish`, to do all the steps that `publish` currently does except for the real `pub publish`, so it would dry-run the publish and then create and push the tag if successful.
  - `--already-tagged`, to skip the step of adding and pushing a tag, and replace it with a check that `HEAD` already has the expected tag.

This set of additions supports a workflow where the current `release` step is changed to use `--tag-for-auto-publish`, and then the separate auto-publish system would publish each package with `... publish --already-tagged --packages=<some package> --exact-match-only`.

See #5005 (comment) for previous discussion/context.

Part of flutter/flutter#126827
LouiseHsu pushed a commit to LouiseHsu/packages that referenced this pull request Mar 7, 2024
Adds the flowing to the tool:
- A new `--exact-match-only` flag to be used with `--packages` to prevent group matching (i.e., a selection like `--packages=path_provider --exact-match-only` would only run on `packages/path_provider/path_provider`, not `packages/path_provider/*`).
- Two new `publish` command flags:
  - `--tag-for-auto-publish`, to do all the steps that `publish` currently does except for the real `pub publish`, so it would dry-run the publish and then create and push the tag if successful.
  - `--already-tagged`, to skip the step of adding and pushing a tag, and replace it with a check that `HEAD` already has the expected tag.

This set of additions supports a workflow where the current `release` step is changed to use `--tag-for-auto-publish`, and then the separate auto-publish system would publish each package with `... publish --already-tagged --packages=<some package> --exact-match-only`.

See flutter#5005 (comment) for previous discussion/context.

Part of flutter/flutter#126827
@stuartmorgan-g
Copy link
Collaborator

@sealesj What's the next step here now that the tool support has landed?

@sealesj
Copy link
Author

sealesj commented Apr 9, 2024

@stuartmorgan Thank you for following up. I've added this to my to-do list and will be working on it soon as a P2 task

@sealesj sealesj self-assigned this Apr 9, 2024
@sealesj sealesj added the team: infra Infrastructure like devicelab, Cirrus, or LUCI. label Apr 9, 2024
@stuartmorgan-g
Copy link
Collaborator

From triage: Is this still in development, or was it impacted by team changes?

@christopherfujino
Copy link
Contributor

From triage: Is this still in development, or was it impacted by team changes?

TIL about this. Gonna close this as I don't plan to implement it. Feel free to re-open if you think there's value and something we should work on this year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team: infra Infrastructure like devicelab, Cirrus, or LUCI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants