#14340 & #13849 - Clean secret ID and improve encrypt/decrypt exception management #14356

pmbrull · 2023-12-12T17:08:09Z

Describe your changes:

Also takes care of this Collate's issue https://github.com/open-metadata/openmetadata-collate/issues/227

We cleaned one internal method that was removing exception messages in some scenarios. Now the exception gives more information on what is the root cause of the encrypt/decrypt error
We are cleaning the secret ID when building it to not include strange characters not supported by AWS SM
We are updating the config to handle prefix and tags when creating the secret in AWS

  prefix: ${SECRET_MANAGER_PREFIX:-""} # Define the secret key ID as /<prefix>/<clusterName>/<key>
  tags: ${SECRET_MANAGER_TAGS:-[]} # Add tags to the created resource, e.g., in AWS. Format is `[key1:value1,key2:value2,...]`

I'll do the docs in a followup PR

I worked on ... because ...

Type of change:

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation>
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

conf/openmetadata.yaml

openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java

harshach · 2023-12-14T00:31:35Z

openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManagerFactory.java

        */
-        secretsManager = NoopSecretsManager.getInstance(clusterName, secretsManagerProvider);
+        secretsManager = NoopSecretsManager.getInstance(secretsManagerProvider, secretsConfig);


can we avoid the switch case, instead use something similar to ENTITY_REPOSITORY_MAP?

mmhh when trying it out it gets a bit ugly actually. We are not initializing all the secret managers just the one being requested by the config, and it would require some further changes on the parent class to allow getting the instances just right.

if it's ok, I'll keep the switch for now

...e/src/test/java/org/openmetadata/service/resources/services/PipelineServiceResourceTest.java

harshach · 2023-12-14T00:33:54Z

openmetadata-service/src/main/java/org/openmetadata/service/secrets/NoopSecretsManager.java

@@ -18,13 +18,14 @@
 public class NoopSecretsManager extends SecretsManager {
  private static NoopSecretsManager instance;

-  private NoopSecretsManager(String clusterPrefix, SecretsManagerProvider secretsManagerProvider) {
-    super(secretsManagerProvider, clusterPrefix);
+  private NoopSecretsManager(SecretsManagerProvider secretsManagerProvider, SecretsConfig secretsConfig) {


Instead of calling this as NoopSecretsManager, lets call it as DBSecretsManager and use the fernet key to encrypt/decrypt the passwords.
We also needs to have config of key/value pairs thats optional and can be passed as part of the config

harshach · 2023-12-14T01:00:59Z

openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java

+                        toEncryptObject,
+                        Fernet.isTokenized(newFieldValue)
+                            ? newFieldValue
+                            : store ? fernet.encrypt(newFieldValue) : newFieldValue,


we need to remove fernet.encrypt from here and make it into DBSecretesManager and except that logic to live inside of these secret managers

github-actions · 2023-12-14T09:42:37Z

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

openmetadata-ui/src/main/resources/ui/src/mocks/IngestionListTable.mock.ts

…able.mock.ts

github-actions · 2023-12-15T10:42:49Z

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	49.59% (24760/49933)	32.37% (9717/30019)	30.83% (2808/9109)

sonarqubecloud · 2023-12-15T14:06:24Z

Quality Gate passed for 'open-metadata-ui'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarqubecloud · 2023-12-15T14:11:49Z

Quality Gate passed for 'open-metadata-ingestion'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
83.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

harshach · 2023-12-17T23:06:11Z

docker/docker-compose-quickstart/docker-compose.yml

@@ -140,7 +140,7 @@ services:
      FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}

      #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}


if someone exported SECRET_MANAGER as noop they will run into issues, worth noting this as backward incompatible change and capture it in our upgrade notes

pmbrull added 4 commits December 12, 2023 12:58

Fix supported characters in SM

fe7cec8

Update SM

54454c4

Fixes

35d8345

Fixes

e90a648

pmbrull requested review from a team as code owners December 12, 2023 17:08

github-actions bot added Ingestion backend safe to test Add this label to run secure Github workflows on PRs labels Dec 12, 2023

Improve class conversion exceptions

ef0c8aa

harshach requested changes Dec 14, 2023

View reviewed changes

harshach reviewed Dec 14, 2023

View reviewed changes

pmbrull added 2 commits December 14, 2023 09:01

Comments

5f2007a

Rename noop to db secrets manager providee

a7ea999

pmbrull added 4 commits December 14, 2023 14:11

Update sm

0bc7bc4

Fix

b48234e

db SM

6914420

db SM

0b59463

pmbrull requested a review from akash-jain-10 as a code owner December 14, 2023 14:24

github-actions bot added the devops label Dec 14, 2023

pmbrull added 2 commits December 14, 2023 17:21

Fix test

b7530a5

UI

2a399e5

pmbrull requested review from karanh37 and chirag-madlani as code owners December 15, 2023 08:14

github-actions bot added the UI UI specific issues label Dec 15, 2023

pmbrull commented Dec 15, 2023

View reviewed changes

openmetadata-ui/src/main/resources/ui/src/mocks/IngestionListTable.mock.ts Outdated Show resolved Hide resolved

pmbrull added 2 commits December 15, 2023 11:24

Update openmetadata-ui/src/main/resources/ui/src/mocks/IngestionListT…

02298a7

…able.mock.ts

Merge branch 'main' into issue-14340

da36bc2

pmbrull added 2 commits December 15, 2023 14:49

update default

2190687

Merge remote-tracking branch 'origin/issue-14340' into issue-14340

1cfe434

akash-jain-10 approved these changes Dec 15, 2023

View reviewed changes

harshach reviewed Dec 17, 2023

View reviewed changes

harshach approved these changes Dec 17, 2023

View reviewed changes

pmbrull merged commit d8984d2 into open-metadata:main Dec 18, 2023
22 of 23 checks passed

pmbrull deleted the issue-14340 branch December 18, 2023 05:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#14340 & #13849 - Clean secret ID and improve encrypt/decrypt exception management #14356

#14340 & #13849 - Clean secret ID and improve encrypt/decrypt exception management #14356

pmbrull commented Dec 12, 2023

harshach Dec 14, 2023

pmbrull Dec 14, 2023

harshach Dec 14, 2023

harshach Dec 14, 2023

github-actions bot commented Dec 14, 2023

github-actions bot commented Dec 15, 2023

sonarqubecloud bot commented Dec 15, 2023

sonarqubecloud bot commented Dec 15, 2023

harshach Dec 17, 2023

#14340 & #13849 - Clean secret ID and improve encrypt/decrypt exception management #14356

#14340 & #13849 - Clean secret ID and improve encrypt/decrypt exception management #14356

Conversation

pmbrull commented Dec 12, 2023

Describe your changes:

Type of change:

Checklist:

harshach Dec 14, 2023

Choose a reason for hiding this comment

pmbrull Dec 14, 2023

Choose a reason for hiding this comment

harshach Dec 14, 2023

Choose a reason for hiding this comment

harshach Dec 14, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 14, 2023

github-actions bot commented Dec 15, 2023

Jest test Coverage

UI tests summary

sonarqubecloud bot commented Dec 15, 2023

Quality Gate passed for 'open-metadata-ui'

sonarqubecloud bot commented Dec 15, 2023

Quality Gate passed for 'open-metadata-ingestion'

harshach Dec 17, 2023

Choose a reason for hiding this comment