vikaskr22 commented on code in PR #910:
URL: https://github.com/apache/ranger/pull/910#discussion_r3071408312
##########
kms/src/main/java/org/apache/hadoop/crypto/key/RangerGoogleCloudHSMProvider.java:
##########
@@ -68,37 +68,36 @@ public RangerGoogleCloudHSMProvider(Configuration conf)
throws Exception {
@Override
public boolean generateMasterKey(String unusedPassword) throws Throwable {
- //The ENCRYPT_DECRYPT key purpose enables symmetric encryption.
- //All keys with key purpose ENCRYPT_DECRYPT use the
GOOGLE_SYMMETRIC_ENCRYPTION algorithm.
- //No parameters are used with this algorithm.
- CryptoKey key = CryptoKey.newBuilder()
- .setPurpose(CryptoKeyPurpose.ENCRYPT_DECRYPT)
- .setVersionTemplate(CryptoKeyVersionTemplate.newBuilder()
- .setProtectionLevel(ProtectionLevel.HSM)
-
.setAlgorithm(CryptoKeyVersionAlgorithm.GOOGLE_SYMMETRIC_ENCRYPTION))
- .build();
-
- // Create the key.
- CryptoKey createdKey = null;
- try {
- createdKey = client.createCryptoKey(this.keyRingName,
this.gcpMasterKeyName, key);
- } catch (Exception e) {
- if (e instanceof AlreadyExistsException) {
- logger.info("MasterKey with the name '{}' already exist.",
this.gcpMasterKeyName);
- return true;
- } else {
+ if (this.masterKeyExists()) {
Review Comment:
@bhaveshamre , One minor comment, Can we try to have one single "return"
statement, like declare one local boolean var say
`boolean isMKGenerated = false`
Then set this to true when MK actually gets created and at last return this
var.
##########
kms/pom.xml:
##########
@@ -103,6 +103,16 @@
<artifactId>jsr305</artifactId>
<version>${jsr305.version}</version>
</dependency>
+ <dependency>
Review Comment:
@bhaveshamre , I want to understand why this new lib is required. As per my
knowledge, `com.google.giava:failureaccess` is part of Guava lib. Are we using
this anywhere directly in the KMS code ?
##########
kms/src/main/java/org/apache/hadoop/crypto/key/RangerGoogleCloudHSMProvider.java:
##########
@@ -221,4 +220,24 @@ private static void updateEnv(String name, String val)
throws ReflectiveOperatio
writeAbleEnvMap.put(name, val);
}
+
+ private boolean masterKeyExists() throws Throwable {
+ if (this.client == null) {
+ throw new RuntimeCryptoException("Google Cloud KMS client is not
initialized; call onInitialization() first.");
+ }
+
+ CryptoKeyName keyName = CryptoKeyName.of(this.gcpProjectId,
this.gcpLocationId, this.gcpKeyRingId, this.gcpMasterKeyName);
+
+ try {
+ CryptoKey cryptoKey = this.client.getCryptoKey(keyName);
+ logger.info("Ranger masterKey present with name: {}",
cryptoKey.getName());
+ return true;
Review Comment:
same review comment as above. Pls try to have one single return statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
