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(cli): IAM differences table printing is broken #12330

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Jan 4, 2021

We were passing (by mistake) an unrecognized property to the table module.

This didn't use to be a problem, but the table module has suddenly
started throwing an "invalid config" error. The cause is a dependency
update that happened today.

Didn't dive into all of the changes, but it stands to reason the
validator has become more strict and where it used to allow additional
properties (probably by accident) it no longer does so.

Undo the passing of the unused property.

This change is a chore instead of a fix because the change
wasn't released yet.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

We were passing (by mistake) an unrecognized property to the `table` module.

This didn't use to be a problem, but the `table` module has suddenly
started throwing an "invalid config" error. The cause is a dependency
update that happened today.

* In #12324, the version of `table` was updated from `6.0.4` to `6.0.7`.
* In [6.0.5](https://github.com/gajus/table/releases/tag/v6.0.5),
  `table` changes the way it does JSON validation of the input
  configuration ob ject.

Didn't dive into all of the changes, but it stands to reason the
validator has become more strict and where it *used* to allow additional
properties (probably by accident) it no longer does so.

Undo the passing of the unused property.

This change is a **chore** instead of a **fix** because the change
wasn't released yet.
@rix0rrr rix0rrr requested a review from a team January 4, 2021 14:25
@rix0rrr rix0rrr self-assigned this Jan 4, 2021
@gitpod-io
Copy link

gitpod-io bot commented Jan 4, 2021

@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jan 4, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 4, 2021
@mergify
Copy link
Contributor

mergify bot commented Jan 4, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 062bf5f into master Jan 4, 2021
@mergify mergify bot deleted the huijbers/iam-table branch January 4, 2021 15:02
@klausbadelt
Copy link
Contributor

@rix0rrr there had been PR #12307 waiting for this... See also my comment about wrapWord which was likely the original intention?

@klausbadelt
Copy link
Contributor

Actually, this should not be chore but fix - I have this issue with 1.82 and 1.81

flochaz pushed a commit to flochaz/aws-cdk that referenced this pull request Jan 5, 2021
We were passing (by mistake) an unrecognized property to the `table` module.

This didn't use to be a problem, but the `table` module has suddenly
started throwing an "invalid config" error. The cause is a dependency
update that happened today.

* In aws#12324, the version of `table` was updated from `6.0.4` to `6.0.7`.
* In [6.0.5](https://github.com/gajus/table/releases/tag/v6.0.5),
  `table` changes the way it does JSON validation of the input
  configuration ob ject.

Didn't dive into all of the changes, but it stands to reason the
validator has become more strict and where it *used* to allow additional
properties (probably by accident) it no longer does so.

Undo the passing of the unused property.

This change is a **chore** instead of a **fix** because the change
wasn't released yet.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@rix0rrr
Copy link
Contributor Author

rix0rrr commented Jan 6, 2021

@klausbadelt are you using Yarn by any chance?

This change should not have made it out according to the GitHub release history, and we shrinkwrap the CLI so you shouldn't be seeing this... unless you use Yarn which doesn't honor shrinkwrapping.

@klausbadelt
Copy link
Contributor

Yes, Yarn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants