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

Rework config storage #986

Merged
merged 4 commits into from
Mar 2, 2018
Merged

Rework config storage #986

merged 4 commits into from
Mar 2, 2018

Conversation

ellismg
Copy link
Contributor

@ellismg ellismg commented Feb 28, 2018

This change reworks the way configuration is stored, adopting the model we discussed previously. In this new model, there is no longer a concept of project settings vs workspace settings and configuration for a stack is now stored in a file next to Pulumi.yaml, named Pulumi.<stack-name>.yaml

I've added code so that on first run, pulumi will try to migrate projects from the old plan to the new plan, which should minimize the pain, but switches like --all and --save are no longer supported.

It may be more approachable to review commit by commit.


For the changelog:

This change updates our configuration model to make it simpler to
understand by removing some features and changing how things are
persisted in files.

Notable changes:

  • We've removed the notion of "workspace" vs "project"
    config. Now, configuration is always stored in a file next to
    Pulumi.yaml named Pulumi.<stack-name>.yaml (the same file we'd
    use for an other stack specific information we would need to persist
    in the future).
  • We've removed the notion of project wide configuration. Every new
    stack gets a completely empty set of configuration and there's no
    way to share common values across stacks, instead the common value
    has to be set on each stack.

Fixes #866
Closes #872
Closes #731

@ellismg ellismg added impact/breaking Fixing this issue will require a breaking change impact/changelog labels Feb 28, 2018
Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

LGTM, other than some minor typos.

Is the expectation that these Pulumi.<stack-name>.yaml files will be .gitignore'd most of the time (unless you have some config you want checked-in)?

cmd/config.go Outdated
}
raw, err := v.Value(d)
if err != nil {
return errors.Wrap(err, "could not decrypt configuation value")
Copy link
Member

Choose a reason for hiding this comment

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

Nit: configuation => configuration


// upgradeConfigutationFiles does an upgrade to move from the old configuration system (where we had config stored) in
// workspace settings and Pulumi.yaml, to the new system where configuration data is stored in Pulumi.<stack-name>.yaml
func upgradeConfigutationFiles() error {
Copy link
Member

Choose a reason for hiding this comment

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

upgradeConfigutationFiles => upgradeConfigurationFiles (and in the comment above)

func upgradeConfigutationFiles() error {
bes, _ := allBackends()

skipedUpgrade := false
Copy link
Member

Choose a reason for hiding this comment

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

Nit: skipedUpgrade => skippedUpgrade

}

// Read a passphrase and confirm it.
// Here, the stack does not have an EcryptionSalt, so we will get a passphrase and create one
Copy link
Member

Choose a reason for hiding this comment

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

Nit: EcryptionSalt => EncryptionSalt

return len(ps.Config) == 0 && ps.EncryptionSalt == ""
}

// Save writes a project defitiniton to a file.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: defitiniton => definition

@ellismg
Copy link
Contributor Author

ellismg commented Feb 28, 2018

Is the expectation that these Pulumi..yaml files will be .gitignore'd most of the time (unless you have some config you want checked-in)?

Yes.

We are going to be changing the configuration model. To begin, let's
take most of the existing stuff and mark it as "deprecated" so we can
keep the existing behavior (to help transition newer code forward)
while making it clear what APIs should not be called in the
implementation of `pulumi` itself.
We had other ways to get at this data, so let's just unify on them.
This change updates our configuration model to make it simpler to
understand by removing some features and changing how things are
persisted in files.

Notable changes:

- We've removed the notion of "workspace" vs "project"
  config. Now, configuration is always stored in a file next to
  `Pulumi.yaml` named `Pulumi.<stack-name>.yaml` (the same file we'd
  use for an other stack specific information we would need to persist
  in the future).
- We've removed the notion of project wide configuration. Every new
  stack gets a completely empty set of configuration and there's no
  way to share common values across stacks, instead the common value
  has to be set on each stack.

We retain some of the old code for the configuration system so we can
support upgrading a project in place. That will happen with the next
change.

This change fixes some issues and allows us to close some
others (since they are no longer possible).

Fixes #866
Closes #872
Closes #731
Migrate configuration from the old model to the new model. The
strategy here is that when we first run `pulumi` we enumerate all of
the stacks from all of the backends we know about and for each stack
get the configuration values from the project and workspace and
promote them into the new file. As we do this, we remove stack
specific config from the workspace and Pulumi.yaml file.

If we are able to upgrade all the stacks we know about, we delete all
global configuration data in the workspace and in Pulumi.yaml as well.

We have a test that ensures upgrades continue to work.
@ellismg
Copy link
Contributor Author

ellismg commented Mar 1, 2018

(Close and re-open to see if that makes the AppVeyor job actually run)

@ellismg ellismg closed this Mar 1, 2018
@ellismg ellismg reopened this Mar 1, 2018
@ellismg ellismg merged commit 96d7f93 into master Mar 2, 2018
@ellismg ellismg deleted the config-refactor branch March 12, 2018 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/breaking Fixing this issue will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants