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

Document migrating aws.s3.Bucket to aws.s3.BucketV2 #5444

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

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Sep 17, 2024

Description

Add a migration guide for moving from aws.s3.Bucket to aws.s3.BucketV2.

Fixes https://github.com/pulumi/home/issues/3630

Adding a new package?

If this pull request adds a new package:

  • The package's schema URL in this PR is correct.
  • The package metadata file, if present, contains:
    • a supported category (one of Cloud, Infrastructure, Network, Database, Monitoring, or Utility).
    • a valid plugin download URL. See Publish Your Package.
    • a description that explains what the package does.
    • a valid logo URL that points to a PNG whose dimensions conform to the others in this repo (e.g., 100x100).
  • The package repo contains an Overview doc (/docs/_index.md) that includes:
    • a brief explanation of what the package is and what it does.
    • at least one representative example in all supported languages.
    • a front-matter property for the layout set to package.
  • The package repo contains an Installation and Configuration doc (/docs/installation-configuration.md) that includes:
    • links to SDKs in all supported languages.
    • a copyable command for installing the resource plugin if necessary.
    • an example of configuring the provider with pulumi config set.
    • an example of configuring the provider with environment variables.
  • The repository has:
    • a version tag prefixed with v that corresponds with a valid GitHub release and published package SDKs
  • A CODEOWNER has reviewed the PR.
  • A member of the @pulumi/docs team has reviewed all documentation.

@t0yv0
Copy link
Member Author

t0yv0 commented Sep 17, 2024

@interurban opening for early review, thanks a lot!

As an aside, I'm working to get examples formatted with the right <chooser> magic and I'm putting together a little script that updates the non-yaml sections automatically via pulumi convert. Would you all be interested in something like this or is there existing machinery to do this? Much appreciated.

Copy link

Your site preview for commit 6812b5d is ready! 🎉

http://registry--origin-pr-5444-6812b5d7.s3-website.us-west-2.amazonaws.com/registry.

Copy link

Your site preview for commit 95a25b7 is ready! 🎉

http://registry--origin-pr-5444-95a25b7f.s3-website.us-west-2.amazonaws.com/registry.

Copy link

Your site preview for commit f7e5595 is ready! 🎉

http://registry--origin-pr-5444-f7e5595e.s3-website.us-west-2.amazonaws.com/registry.

Copy link

Your site preview for commit 50358fc is ready! 🎉

http://registry--origin-pr-5444-50358fc1.s3-website.us-west-2.amazonaws.com/registry.

@t0yv0
Copy link
Member Author

t0yv0 commented Sep 17, 2024

Looks like something with choosers inside lists is not happy:

/home/runner/work/registry/registry/themes/default/content/registry/packages/aws/how-to-guides/bucketv2-migration.md:
Line 2417: Ordered list item prefix [Expected: 1; Actual: 6; Style: 1/2/3].
Line 2420: Ordered list item prefix [Expected: 2; Actual: 7; Style: 1/2/3].
Line 2422: Ordered list item prefix [Expected: 3; Actual: 8; Style: 1/2/3].

Copy link

Your site preview for commit f9a1f60 is ready! 🎉

http://registry--origin-pr-5444-f9a1f600.s3-website.us-west-2.amazonaws.com/registry.

2. Perform `pulumi up` to replace (delete and re-create) the buckets in AWS.

If replacement is not acceptable, it is possible to perform a manual migration with `pulumi import` (see Avoiding
replacement).
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to link to the section like this: [Avoiding Replacement](#avoiding-replacement). Not 100% sure though whether Hugo supports that syntax

Copy link
Member Author

Choose a reason for hiding this comment

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

It's been failing builds. Looks like this only works in GitHub markdown. @interurban might have some pointers here.




As a bonus, the policy can now more easily refer to the concrete name of the bucket.
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be worth explaining the benefit of not having cyclic deps in a bit more detail in the beginning. Gives some more insights into why this is a good thing for users.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a difficult sell. I tried adding a Context section at the bottom, but not sure it belongs, perhaps this sentence does not belong either. Primary motivation for users to do this is to get onto a supported version because they have to.

I've checked the style of https://registry.terraform.io/providers/hashicorp/aws/latest/docs/guides/version-4-upgrade#s3-bucket-refactor FWIW it's very technical to-the-point and does not explain any design considerations as to why.

This will only be available as an output. If your program specified this as an input, simply remove it. The input was
ignored by prior versions of the provider and was exposed in error.

### other inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

How were the props with explicit instructions chosen? I guess they're the most used ones?

I'm wondering if we should have a single example with all options (might not be possible if some settings are mutually exclusive) and show how to migrate that instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I've just taken one of the more commonly used properties. It's not a bad idea, try running through this which might prove instructive.

against the actual cloud account. While the details will vary depending on your use case, this procedure generally
involves the following steps:

- Find URNs for legacy Bucket Pulumi resources using `pulumi stack export`
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to extract those with jq, right? Could be a nice addition to add that here

Copy link

Your site preview for commit a26fefd is ready! 🎉

http://registry--origin-pr-5444-a26fefd1.s3-website.us-west-2.amazonaws.com/registry.

Copy link

Your site preview for commit f172554 is ready! 🎉

http://registry--origin-pr-5444-f1725541.s3-website.us-west-2.amazonaws.com/registry.

@t0yv0
Copy link
Member Author

t0yv0 commented Sep 18, 2024

@flostadler I went ahead and took a larger bucket example for a spin through the import-based migration procedure. It worked but highlighted a few extra steps, this was useful.

For alternative upgrade procedures.

I have filed pulumi/pulumi-aws#4471 as this seems to have regressed.

I've also had a slack conversation with a user trying this today and filed pulumi/pulumi-aws#4470 - sounds like we could do a bit more possibly, depending on interest here.

Copy link

Your site preview for commit e7f99a0 is ready! 🎉

http://registry--origin-pr-5444-e7f99a0d.s3-website.us-west-2.amazonaws.com/registry.

@t0yv0 t0yv0 marked this pull request as ready for review September 18, 2024 20:59
@georgberky
Copy link

georgberky commented Sep 19, 2024

I've also had a slack conversation with a user trying this today and filed pulumi/pulumi-aws#4470 - sounds like we could do a bit more possibly, depending on interest here.

The confusing things for me were to find the old migration guide and

  • the full explanation being in a video instead of in text
  • the video showing a technique that my Typescript compiler forbids (redeclaring a variable)
  • the guide not being too clear that you have to declare a v2 resource next to the v1 resource instead of replacing v1 with v2 even though they have the same Pulumi resource name
  • the whole mechanism not working anymore
  • an IaC tool trying to delete my resource (luckily failing because the old bucket wasn't empty, but this doesn't raise confidence)

If the mechanism doesn't work anymore, remove the guide or clearly state on top that it stopped working with v12345 of Pulumi.

@t0yv0
Copy link
Member Author

t0yv0 commented Sep 19, 2024

Thanks for your feedback @georgberky that's a good point; we don't edit old blog content but we have a way to marking it obsolete and pointing to new material, I'll make sure we do this to https://www.pulumi.com/blog/announcing-v5.0.0-of-the-pulumi-aws-provider/#can-i-migrate-from-awss3bucket-to-awss3bucketv2

`BucketV2` inputs such as `policy` and `accelerationStatus` are to be replaced by side-by-side resources
`aws.s3.BucketPolicy` and `aws.s3.BucketAccelerateConfigurationV2`.

2. Perform `pulumi up` to replace the buckets in AWS. **WARNING**: replacing the buckets will delete and re-create them,

Choose a reason for hiding this comment

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

I'm wondering if we should flip this and mention replacing buckets as the alternative. I think 99% of buckets won't be able to be replaced so we should just go with that as the default.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right.. that seems to be really true. I'll incorporate.

import * as pulumi from "@pulumi/pulumi";
import * as aws from "@pulumi/aws";

const myBucket = new aws.s3.Bucket("my-bucket", {serverSideEncryptionConfiguration: {

Choose a reason for hiding this comment

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

Suggested change
const myBucket = new aws.s3.Bucket("my-bucket", {serverSideEncryptionConfiguration: {
const myBucket = new aws.s3.Bucket("my-bucket", {
serverSideEncryptionConfiguration: {

In situations when replacing the bucket in the AWS account is not acceptable, it is possible to perform a manual
migration that changes Pulumi program and state to track buckets using the new resource without executing any changes
against the actual cloud account. While the details will vary depending on your use case, this procedure generally
involves the following steps:

Choose a reason for hiding this comment

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

I think as far as manual steps go this is not bad and as good as we can do 👍🏻

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.

4 participants