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

cmd: configuration rework #296

Closed
wants to merge 44 commits into from
Closed

Conversation

bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented Jul 2, 2018

🎟️ Ticket(s): Closes #101, closes #350


👷 Changes

This PR splits configuration into two separate files:

  • inertia.toml, which contains basic project configuration, and should be committed to source.

  • inertia.remotes, which contains remote configuration, and should not be committed to source.

This opens the door for more project-specific configuration that can be source-controlled, as well as the ability to move configuration currently in inertia.remotes out of the project directory into a safer, centralized location to avoid accidental commits. For now, both will live in the user's project directory.

This PR also includes some miscellaneous tests.

The following is no longer valid - see updates for moving inertia.remotes out of the project directory into ~/.inertia/$PROJECT.remotes: #296 (comment), #296 (comment)

🔦 Testing Instructions

$> make cli
$> inertia --project example.inertia.toml --remotes example.inertia.remotes production

🍧 Example

# inertia.toml
version = "latest"
project-name = "inertia"
build-type = "dockerfile"
build-file-path = "Dockerfile"
# inertia.remotes
version = "latest"

[remotes]
  [remotes.production]
    IP = "1.2.3.4"
    user = "root"
    pemfile = "/Users/robertlin/.ssh/id_rsa"
    branch = "master"
    ssh_port = "22"
    [remotes.production.daemon]
      port = "4303"
      token = "asdfasdf"
      webhook_secret = "9oC_5rfFcx2es4NLonMP2leOlJARkmu404EAXHXR4CI="
  [remotes.staging]
    IP = "4.3.2.1"
    user = "bleh"
    pemfile = "/Users/robertlin/.ssh/id_rsa"
    branch = "dev"
    ssh_port = "22"
    [remotes.staging.daemon]
      port = "4303"
      token = "asdfasdf"
      webhook_secret = "7dtg2lmTHF5EDBUgIpdu0zuERqs9Emn8rd3NCh5EiMU="

@bobheadxi bobheadxi added pr: finalized needs review and final approval breaking change this change is backwards-incompatible labels Jul 2, 2018
@bobheadxi bobheadxi requested review from bfbachmann, brian-nguyen and a team July 2, 2018 22:20
@coveralls
Copy link

coveralls commented Jul 2, 2018

Pull Request Test Coverage Report for Build 1123

  • 91 of 249 (36.55%) changed or added relevant lines in 11 files are covered.
  • 13 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.7%) to 41.07%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/env.go 0 3 0.0%
cmd/util.go 7 10 70.0%
cmd/users.go 0 4 0.0%
cmd/provision.go 0 6 0.0%
cfg/toml.go 0 11 0.0%
cmd/root.go 31 43 72.09%
cmd/remote.go 0 12 0.0%
cmd/config.go 0 13 0.0%
local/storage.go 9 22 40.91%
cmd/deploy.go 1 16 6.25%
Files with Coverage Reduction New Missed Lines %
cfg/config.go 4 46.76%
daemon/inertiad/webhook.go 9 0.0%
Totals Coverage Status
Change from base Build 1035: -0.7%
Covered Lines: 1458
Relevant Lines: 3550

💛 - Coveralls

@bobheadxi bobheadxi requested a review from bwdmonkey July 4, 2018 02:01
@PiggySpeed
Copy link
Contributor

@bobheadxi so is it up to the user to add inertia.remotes to .gitignore, or remember to avoid staging it each time? Doesn't seem very user-friendly? Maybe, as you said, we'll try to find a better solution in the future.

@bobheadxi
Copy link
Member Author

@chadlagore and I discussed this quite a bit way back in the day - one way would be to quietly slip in a gitignore rule for inertia.remote. Personally I've never seen a tool do this - though neither have I seen a tool output sensitive information into the project directory 😋

@bobheadxi
Copy link
Member Author

I could expand the scope of this PR to include a move of sensitive information into the user's home directory, or add something that adds a gitignore rule. I'm not a big fan of the latter. Any thoughts @PiggySpeed ?

@PiggySpeed
Copy link
Contributor

@bobheadxi I'm more in favour of extending the task to move it to the user's home directory. Somewhere that the user can't possibly make a mistake committing the wrong file. With the names so similar: inertia.toml and inertia.remotes, it's a mistake waiting to happen.

@bobheadxi bobheadxi added pr: wip in progress but seeking feedback and removed pr: finalized needs review and final approval labels Jul 5, 2018
@bobheadxi
Copy link
Member Author

bobheadxi commented Jul 10, 2018

Current plan:

  • Remote configuration in ~/ (HOME on unix, HOMEPATH on windows), keyed by file name - this is project. There's some contention on whether this is ok, but AWS puts their config there, so I think it's safe for us to follow suit
  • CLI options to specify either project name, but NOT a remote config file path cuz thats too many choices and makes it hard to get the project name
  • inertia.toml will be parsed by daemon and not the client - on first deploy, Inertia will use inertia [remote] send in the background to send the config to the deployment. This should keep the file there until the user commits inertia.toml to source control.

Usage patterns can look like this:

$ cd /any/directory
$ inertia --project bumper my_vps up

This opens the door for configuration for #311, #309, #112, #78, #194, and more

A little bit of inspiration from https://up.docs.apex.sh/#configuration

Thoughts, @PiggySpeed @leesw98 @brian-nguyen ?

@bobheadxi bobheadxi requested a review from bwdmonkey July 21, 2018 20:55
Copy link
Member

@bwdmonkey bwdmonkey left a comment

Choose a reason for hiding this comment

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

Doesn't work 0/10

@bobheadxi bobheadxi added the breaking change this change is backwards-incompatible label Jul 23, 2018
@bobheadxi bobheadxi removed the pr: wip in progress but seeking feedback label Jul 25, 2018
@bobheadxi
Copy link
Member Author

bobheadxi commented Jul 25, 2018

This PR just needs some final live testing, QA, and edge case testing - I've pushed up a canary image for this branch (21add01)

@bobheadxi bobheadxi added pr: wip in progress but seeking feedback and removed pr: finalized needs review and final approval labels Jul 29, 2018
@bobheadxi bobheadxi added this to In progress in Project Deployment Oct 29, 2018
@bobheadxi bobheadxi added won't fix sorry! and removed pr: wip in progress but seeking feedback labels Nov 17, 2018
@bobheadxi
Copy link
Member Author

bobheadxi commented Nov 17, 2018

won't be merging this PR but eventually i rewrite this whole shebang using https://github.com/spf13/viper which now supports config writeback and clean up the mess of daemonside config initialization... leaving this open so i can reference it later tho

Other notes for self:

@bobheadxi bobheadxi changed the title Configuration rework cmd: configuration rework Jan 6, 2019
@bobheadxi
Copy link
Member Author

Closing this, in favour of #577

@bobheadxi bobheadxi closed this Feb 21, 2019
Project Deployment automation moved this from In progress to Done Feb 21, 2019
@bobheadxi bobheadxi deleted the client/#101-config-rework branch February 26, 2019 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change this change is backwards-incompatible won't fix sorry!
Projects
Development

Successfully merging this pull request may close these issues.

Sent files get deleted upon deployment init config: improve format of inertia configuration
4 participants