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

chore: fix allow only CFN-ready values for task run flags; change logging logic #2904

Merged
merged 79 commits into from
Oct 12, 2021

Conversation

huanjani
Copy link
Contributor

@huanjani huanjani commented Oct 8, 2021

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

iamhopaul123 and others added 30 commits August 12, 2021 23:19
<!-- Provide summary of changes -->
https://github.com/aws/copilot-cli/security/dependabot/cf-custom-resources/package-lock.json/path-parse/open
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Adds a "deployed SNS topic" selector to the `select` package and some data structures to accompany it. Uses the `deploy store` under the hood and relies on resource group tagging API, so it can only be used on one environment at a time. 

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…behaviors (aws#2738)

This PR is part of the second step (writing units test) to address aws#2492. It:
1. Upgrades `mergo`
2. Fixes code by implementing custom transformers so that all `applyEnv` tests that have been passing are still passing

Next: 
1. Change pointer struct to struct and remove `pStruct` transformers
2. Fix bugs (tests that have been failing and commented out in previous PRs (aws#2700 ~ aws#2734)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Bumps [golang.org/x/mod](https://github.com/golang/mod) from 0.4.2 to 0.5.0.
- [Release notes](https://github.com/golang/mod/releases)
- [Commits](golang/mod@v0.4.2...v0.5.0)

---
updated-dependencies:
- dependency-name: golang.org/x/mod
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.40.17 to 1.40.22.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Changelog](https://github.com/aws/aws-sdk-go/blob/main/CHANGELOG.md)
- [Commits](aws/aws-sdk-go@v1.40.17...v1.40.22)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
<!-- Provide summary of changes -->
Continued from aws#2703. 

Refactor described [here](https://github.com/aws/copilot-cli/pull/2703/files#r683800143) to be implemented in a future PR.

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Part of aws#2588.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Add `--force` flag for `svc deploy` and `deploy`, allowing users to force deploy their ECS service. Fixes aws#2597. E2E test and doc will be added in an upcoming PR.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
As mentioned in aws#2744, refactors describer structs to remove redundancy.

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…ws#2750)

<!-- Provide summary of changes -->
Part of aws#2588
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Subsumes aws#2704. 

Also refactors SNS selector to a single prompt, where we only ask the customer to select topics that are deployed across all existing environments.

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
This PR This PR is part of the second step (writing units test) to address aws#2492. It:
1. Fix previously found `ApplyEnv` bugs by implementing transformers needed
2. Refactor `transform.go`

Previous: aws#2738 
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…s#2757)

Bumps [github.com/AlecAivazis/survey/v2](https://github.com/AlecAivazis/survey) from 2.2.16 to 2.3.0.
- [Release notes](https://github.com/AlecAivazis/survey/releases)
- [Commits](AlecAivazis/survey@v2.2.16...v2.3.0)

---
updated-dependencies:
- dependency-name: github.com/AlecAivazis/survey/v2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.15.0 to 1.16.0.
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.15.0...v1.16.0)

---
updated-dependencies:
- dependency-name: github.com/onsi/gomega
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.40.22 to 1.40.27.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Changelog](https://github.com/aws/aws-sdk-go/blob/main/CHANGELOG.md)
- [Commits](aws/aws-sdk-go@v1.40.22...v1.40.27)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
<!-- Provide summary of changes -->
Adds the --yes flag to `copilot svc exec` docs.

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->
Fixes aws#2760

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…ule (aws#2756)

This PR:
1. unmarshal/marshal CFN template to/from YAML nodes.
2. parse node from rule.
3. add a unit test for the exported function `CloudformationTemplate`.

Part of aws#2588

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Addressing: aws#2756 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…2767)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Part of aws#2588. Work on the basis of aws#2756.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
`FIFOOrBool` is a composite field and need custom merge logic.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…ws#2770)

Previously, any non-App Runner service threw out the log statement about the platform field in the manifest. 
Now, this happens only when the platform has been redirected _and_ a non-App Runner service has been created.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
)

<!-- Provide summary of changes -->

<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
<!-- Provide summary of changes -->
Part of aws#2588.
<!-- Issue number, if available. E.g. "Fixes #31", "Addresses #42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
…nifest (aws#2775)

⚰️

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
@huanjani huanjani requested a review from a team as a code owner October 8, 2021 17:35
@huanjani huanjani requested review from Lou1415926 and removed request for a team October 8, 2021 17:35
Copy link
Contributor

@efekarakus efekarakus left a comment

Choose a reason for hiding this comment

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

Looks great! just small suggestions

Comment on lines 91 to 96
func ValidOSs() []string {
return []string{osWindowsServer2019Core, osWindowsServer2019Full, osLinux}
}

// ValidArchs returns all valid CFN-friendly architectures.
func ValidArchs() []string {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason why you prefered functions instead of exporting variables? I think this is neat it allows the values to be immutable 👍

@efekarakus efekarakus changed the title fix: allow only CFN-ready values for task run flags; change logging logic chore: fix allow only CFN-ready values for task run flags; change logging logic Oct 12, 2021
Copy link
Contributor

@efekarakus efekarakus left a comment

Choose a reason for hiding this comment

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

Looks good! Just a tiny log request

}
if o.wkldType != manifest.RequestDrivenWebServiceType {
log.Warning("See 'platform' field in your manifest.\n")
log.Warningf("Your architecture type %s is currently unsupported. Setting platform %s instead.\n", color.HighlightCode(detectedArch), redirectedPlatform)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be consistent with job init?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because job init doesn't have to weed out RDWS.

@huanjani huanjani merged commit 320f3e0 into aws:feat/pencere Oct 12, 2021
@huanjani huanjani deleted the task-run-fixes branch October 27, 2021 00:50
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.

8 participants