search

aws-amplify / amplify-cli

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development.
Apache License 2.0
2.81k stars 820 forks source link

amplify/.config should be in .gitignore? #38

Closed jadbox closed 5 years ago

jadbox commented 5 years ago

I noticed the config files under amplify/.config/ often refer to full directory paths and is not suited to be committed into the project. I then noticed that amplify-cli's own .gitignore has an entry to ignore this directory (see below). Should amplify init add this path to the project .gitignore file... as the command does add other ignore directives during init.

https://github.com/aws-amplify/amplify-cli/blob/master/.gitignore

kaustavghosh06 commented 5 years ago

Hey Jonathan. So if you would want to share the configs at another location you would need to share the amplify dir (via version control) and run the ‘amplify configure project’ command. That would setup the .config dir for you in another machine based on that specific machines configuration. You would need to run this from the root of your app where you have copied the amplify dir at.

jadbox commented 5 years ago

Thanks, but shouldn't the .config folder be put into the .gitignore to prevent user updates to that file from overriding another person's generated .config? Like, if I make a change to the .config content and commit into the git, then Bob is going to have his settings overwritten with variables related to my computer on his machine on a git pull. It would make sense for amplify-cli init command to add an extry to .gitignore to ensure .config doesn't get committed.

SagarMhatre commented 5 years ago

I have made the below entries manually to the .gitignore files. In a future release, aws amplify could make similiar entries to the .gitignore. 

#awsamplifyjs
amplify/\#current-cloud-backend
aws-info.json
project-config.json
aws-exports.*
amplify/backend/function/*/src/node_modules/
amplify/backend/function/*/dist/
jalooc commented 5 years ago

.gitignore used to be updated when suing awsmobile. I'd expect the same behaviour from awsamplify as it's successor.

stevemao commented 5 years ago

Thanks @SagarMhatre for sharing your gitignore. However, project-config.json cannot be completely ignored because

"javascript": {
    "framework": "react",
    "config": {
      "SourceDir": "frontend/src",
      "DistributionDir": "frontend/dist",
      "BuildCommand": "npm run-script build",
      "StartCommand": "npm run-script start"
    }
  },

part should be commited. So a new computer doesn't need to be specified where aws-exports.js should be exported to in my frontend source.

CodySwannGT commented 5 years ago

But.... it has to be ignored because it has machine-specific information in there: 

{
    "projectName": "client",
    "projectPath": "/Users/cody/workspace/javascript/serverless/shudi2.0/client",
    "defaultEditor": "code",
    "frontendHandler": {
        "javascript": "/Users/cody/.nvm/versions/node/v10.11.0/lib/node_modules/@aws-amplify/cli/node_modules/amplify-frontend-javascript"
    },
    "javascript": {
        "framework": "react-native",
        "config": {
            "SourceDir": "/",
            "DistributionDir": "/",
            "BuildCommand": "npm run-script build",
            "StartCommand": "npm run-script start"
        }
    },
    "providers": {
        "awscloudformation": "/Users/cody/.nvm/versions/node/v10.11.0/lib/node_modules/@aws-amplify/cli/node_modules/amplify-provider-awscloudformation"
    }
}
stevemao commented 5 years ago

Yes. This piece of information should be separated into a different file.

wadim commented 5 years ago

Wow, this is cutting edge stuff. Does anyone have a functioning .gitignore example?

nicosommi commented 5 years ago

I'm also confused on what should be pushed to version control. When setting up the auth service I have an entry that looks like sensitive information to be in a repo (not 100% sure)

File: amplify/#current-cloud-backend/amplify-meta.json Entry: "auth.cognitoFakeObjectId123.AppClientSecret"

What is the recommended way to go?

flowirtz commented 5 years ago

related to #233 and #104

any updates on this @kaustavghosh06? Should all elements in amplify/ be committed to source control or is there sensitive information in there like @nicosommi supposed?

undefobj commented 5 years ago

Specifically the the point of the App Client Secret that is created for the awsconfiguration.json with native apps, this is strictly for Cognito endpoint protections (such as throttling, etc.) and not related to the actual AuthN or AuthZ flows. Cognito uses SRP and client secrets are not leveraged. So while there is no security issue here there is also no good reason not to give the capability to put this in your .gitignore. @kaustavghosh06 let's discuss the best way to add this.

wadim commented 5 years ago

This is just a guess since I don't have any better information.

Please correct me if anything seems incorrect or misleading.

This is what I added to .gitignore based on my very limited insight into how these are used. Nonetheless, it has made it bearable to work with amplify under a git repo.

# Amplify section

# current backend as deployed (presumably used for change detection)
/amplify/\#current-cloud-backend

# config which is partly portable and partly machine-specific ¯\_(ツ)_/¯
/amplify/.config

# information about currently used amplify resources
# (not sure if this should be ignored or not but it changes often)
/amplify/backend/amplify-meta.json

# I am ignoring the function dists too
/amplify/backend/function/*/dist

# ...and the function node_modules
/amplify/backend/function/*/src/node_modules
PaulBurridge commented 5 years ago

I have done a fair amount lot of testing around this and discovered the following with Amplify version 0.1.31:

Using the amplify configure project command which is described in the documentation as for use "When sharing your backend infrastructure code with multiple team members working on different machines" fails unless you have the following in place:

./amplify
./amplify/backend
./amplify/backend/amplify-meta.json
./amplify/.config
./amplify/.config/project-config.json
./amplify/#current-cloud-backend
./amplify/#current-cloud-backend/amplify-meta.json

This is at odds with the line "Note: The project-config.json file has your local system-specific paths and shouldn’t be checked in to your source control if sharing the project with another team member working on another system." (from https://aws-amplify.github.io/docs/cli/init) as without it the command amplify configure project will not work.

The problem with the current process would be users overwriting each others config and having to use amplify configure project after pulling down the latest code from another user. It is essential that local workstation / user configuration is separated from the general project configuration and ideally adding a cli command that would allow local configuration to be generated (without offering changes to general shared configuration).

kaustavghosh06 commented 5 years ago

@PaulBurridge-kcom We're actively working right now on making inter-team and intra-team workflows, and CI/CD workflows better as a part of the Amplify CLI which should solve all the above-mentioned issues that you've mentioned.

michaelcaterisano commented 5 years ago

Any update on this, @kaustavghosh06?

kaustavghosh06 commented 5 years ago

@michaelcaterisano Just to give you an idea, we're close to complete with the beta version of team-workflows and CI/CD flows as part of the Amplify CLI and working actively on our documentation for this release. As soon as we launch it, I'll update this thread (expect an announcement sometime next week).

framled commented 5 years ago

@kaustavghosh06 some news about this?

MorelSerge commented 5 years ago

@framled See https://aws-amplify.github.io/docs/cli/multienv?sdk=js

kaustavghosh06 commented 5 years ago

With the release of the multienv feature-set, I assume we can close this issue. Please feel free to re-open this issue if you've further questions.

artista7 commented 5 years ago

Hey @kaustavghosh06, I have a doubt. If we don't share amplify-meta.json from backend, and if I update the backend, amplify status will show no updates (which is fine). But if another team member, does amplify env pull (to fetch latest cloud-backend with latest amplify-meta), if he does amplify status, some resources still shows, update needed. Similar problem occurs while pushing changes into qa, or staging or prod repo. The changes are deployed, but our local repo, shows update needed.

Correct me if im wrong.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.