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.

Sign up for GitHub

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

Jump to bottom

chore(deps): update dependency com.google.cloud:google-cloud-kms to v1.39.0 #3235

Merged
merged 1 commit into from
Jun 25, 2020
Merged

chore(deps): update dependency com.google.cloud:google-cloud-kms to v1.39.0 #3235

merged 1 commit into from
Jun 25, 2020

Conversation

renovate-bot
Copy link
Contributor

This PR contains the following updates:

Package Update Change
com.google.cloud:google-cloud-kms minor 1.38.1 -> 1.39.0

Release Notes

googleapis/java-kms

v1.39.0

Compare Source

Features
  • deps: adopt flatten plugin and google-cloud-shared-dependencies (#​203) (4dbf4db)
Bug Fixes
Dependencies
  • update dependency com.google.api:api-common to v1.9.1 (#​175) (25969ce)
  • update dependency com.google.api:api-common to v1.9.2 (#​183) (4e6d75f)
  • update dependency com.google.api.grpc:proto-google-common-protos to v1.18.0 (#​164) (5cff5b7)
  • update dependency com.google.cloud:google-cloud-core to v1.93.5 (#​176) (a042993)
  • update dependency com.google.http-client:google-http-client-bom to v1.35.0 (#​159) (3f7d1da)
  • update dependency com.google.protobuf:protobuf-java to v3.12.0 (#​168) (4e9597f)
  • update dependency com.google.protobuf:protobuf-java to v3.12.2 (#​173) (c9855e4)
  • update dependency io.grpc:grpc-bom to v1.29.0 (#​152) (b351d28)
  • update dependency io.grpc:grpc-bom to v1.30.0 (#​188) (2d8865f)
  • update dependency org.threeten:threetenbp to v1.4.4 (#​158) (6b68afe)
Documentation
  • change relative URLs to absolute URLs to fix broken links. (#​184) (b7a6bcd)
1.38.1 (2020-04-20)
Dependencies
  • update dependency com.google.guava:guava-bom to v29 (#​135) (8e61429)

Renovate configuration

📅 Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻️ Rebasing: Never, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

  • If you want to rebase/retry this PR, check this box

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate-bot renovate-bot requested a review from a team June 24, 2020 00:53
@forking-renovate forking-renovate bot added the automerge Merge the pull request once unit tests and other checks pass. label Jun 24, 2020
@trusted-contributions-gcf trusted-contributions-gcf bot added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 24, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@gcf-merge-on-green
Copy link
Contributor

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 24, 2020
@lesv
Copy link
Contributor

lesv commented Jun 24, 2020

Java 11

@chingor13 @stephaniewang526 @sethvargo PTAL - same problem as secrets manager

------------------------------------------------------------
- testing kms
------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveLabels.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveRotation.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyUpdateLabels.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/EnableKeyVersion.java:[25,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/DisableKeyVersion.java:[25,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyAddRotation.java:[29,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveLabels.java:[53,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.UpdateKeyRemoveLabels
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveRotation.java:[58,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.UpdateKeyRemoveRotation
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyUpdateLabels.java:[64,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.UpdateKeyUpdateLabels
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/EnableKeyVersion.java:[62,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.EnableKeyVersion
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/DisableKeyVersion.java:[62,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.DisableKeyVersion
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyAddRotation.java:[76,29] cannot find symbol
  symbol:   variable FieldMaskUtil
  location: class kms.UpdateKeyAddRotation
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project kms-samples: Compilation failure: Compilation failure:
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveLabels.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveRotation.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyUpdateLabels.java:[24,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/EnableKeyVersion.java:[25,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/DisableKeyVersion.java:[25,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyAddRotation.java:[29,32] package com.google.protobuf.util does not exist
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveLabels.java:[53,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.UpdateKeyRemoveLabels
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyRemoveRotation.java:[58,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.UpdateKeyRemoveRotation
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyUpdateLabels.java:[64,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.UpdateKeyUpdateLabels
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/EnableKeyVersion.java:[62,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.EnableKeyVersion
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/DisableKeyVersion.java:[62,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.DisableKeyVersion
[ERROR] /tmpfs/src/github/java-docs-samples/kms/src/main/java/kms/UpdateKeyAddRotation.java:[76,29] cannot find symbol
[ERROR]   symbol:   variable FieldMaskUtil
[ERROR]   location: class kms.UpdateKeyAddRotation
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

 Testing failed: Maven returned a non-zero exit code.

@sethvargo
Copy link
Member

@lesv Is that class deprecated or removed?

1 similar comment
@sethvargo
Copy link
Member

@lesv Is that class deprecated or removed?

@lesv lesv added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@stephaniewang526
Copy link
Contributor

@lesv Is that class deprecated or removed?

Upon checking java-kms code, FieldMaskUtil class has been removed from the client -- the samples need to be rewritten/removed.

@sethvargo
Copy link
Member

@stephaniewang526 that class is used everywhere across all the client libraries. Why was it removed? It's also the only reasonable way to build a field mask as far as I know.

@stephaniewang526
Copy link
Contributor

@sethvargo actually I'm checking and seeing that FieldMaskUtil is still in protobuf-java-util jar and java-kms is pulling that artifact in. let's re-run the tests again to see if it picks it up.

@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 24, 2020
@lesv
Copy link
Contributor

lesv commented Jun 24, 2020

This keeps failing and has been run 3 times now.

@sethvargo
Copy link
Member

Since this is affecting multiple packages, I'm relatively confident it's not unique to the java or secret manager samples, and is likely an indication of a larger, upstream breaking change.

@chingor13
Copy link
Member

This is happening because protobuf-java-utils was changed from a compile dependency to runtime dependency (it's not used at compile time by google-cloud-kms).

The flatten-maven-plugin likely has something to do with this (@stephaniewang526).

In any case, these sample are using protobuf-java-utils directly and thus should be explicitly declaring that dependency.

@sethvargo
Copy link
Member

In any case, these sample are using protobuf-java-utils directly and thus should be explicitly declaring that dependency.

Regardless, this is a breaking change and should be reverted.

@chingor13
Copy link
Member

It's not a breaking change as that class is not part of the google-cloud-kms library and is not necessary to run the library. Removing an unnecessary dependency is actually a bugfix. google-cloud-kms should not force applications to install libraries that are not necessary.

The reason it looks like a breaking change is that this sample is relying on a transitive dependency when it should be declared explicitly.

@sethvargo
Copy link
Member

At t0, it worked. At t1, it's broken. Nothing in this repository has changed between t0 and t1. That's definitionally a breaking change. I'm not an expert in the Java ecosystem, but what caused the dependency to be dropped?

@sethvargo
Copy link
Member

sethvargo commented Jun 25, 2020 •
edited
Loading

As a user, previously I added the following to my pom.xml:

<dependency>
  <groupId>com.google.cloud</groupId>
  <artifactId>google-cloud-kms</artifactId>
  <version>1.38.1</version>
</dependency>

And I had access to the FieldMaskUtil class. Upon upgrading to 1.39.0 (a minor increase), I no longer have access to that class. This, to me, is a breaking change. All our customers now have to update their pom.xml files to add this additional dependency. As far as I know, this was not communicated broadly, and this feels very inappropriate in a minor version bump.

@lesv lesv added api: cloudkms Issues related to the Cloud Key Management Service API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jun 25, 2020
@lesv
Copy link
Contributor

lesv commented Jun 25, 2020

@chingor13 If you remove something from the library, you should bump the major version.

@elharo
Copy link
Contributor

elharo commented Jun 25, 2020

It doesn't seem anything was removed from the library. This sample was depending on transitive dependencies, which is always a bad idea. In google3 this isn't allowed at all, and blaze would fail the build. Maven isn't quite so strict, though there might be an enforcer rule that can check this.

@stephaniewang526
Copy link
Contributor

stephaniewang526 commented Jun 25, 2020 •
edited
Loading

protobuf-java-util was brought in transitively through gax-grpc before, that's why samples did not have to declare directly.

[INFO] |  +- com.google.api:gax-grpc:jar:1.56.0:compile
[INFO] |  |  +- com.google.auth:google-auth-library-credentials:jar:0.20.0:compile
[INFO] |  |  +- io.grpc:grpc-netty-shaded:jar:1.28.1:compile
[INFO] |  |  |  \- io.grpc:grpc-core:jar:1.28.1:compile (version selected from constraint [1.28.1,1.28.1])
[INFO] |  |  |     +- com.google.code.gson:gson:jar:2.8.6:compile
[INFO] |  |  |     +- com.google.android:annotations:jar:4.1.1.4:compile
[INFO] |  |  |     \- io.perfmark:perfmark-api:jar:0.19.0:compile
[INFO] |  |  \- io.grpc:grpc-alts:jar:1.28.1:compile
[INFO] |  |     +- io.grpc:grpc-grpclb:jar:1.28.1:compile
[INFO] |  |     |  \- com.google.protobuf:protobuf-java-util:jar:3.11.0:compile

Since samples/ is using it, it should directly declare it as compile scope dep.

No breaking change was made on the client library side -- this exposes a problem on the consuming side.

@stephaniewang526 stephaniewang526 mentioned this pull request Jun 25, 2020
Merged
@stephaniewang526 stephaniewang526 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 25, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 25, 2020
@stephaniewang526 stephaniewang526 merged commit fbde9a7 into GoogleCloudPlatform:master Jun 25, 2020
@renovate-bot renovate-bot deleted the renovate/com.google.cloud-google-cloud-kms-1.x branch June 25, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv approved these changes

@sethvargo sethvargo sethvargo approved these changes

Assignees

@sethvargo sethvargo

@elharo elharo

@daniel-sanche daniel-sanche

Labels
api: cloudkms Issues related to the Cloud Key Management Service API. cla: yes This human has signed the Contributor License Agreement. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@renovate-bot @lesv @sethvargo @stephaniewang526 @chingor13 @elharo @daniel-sanche @googlebot @averikitsch @kokoro-team