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

Adding implementation for separate ui database user #225

Merged
merged 3 commits into from
Jan 7, 2023
Merged

Adding implementation for separate ui database user #225

merged 3 commits into from
Jan 7, 2023

Conversation

brandtkeller
Copy link
Contributor

Closes #224

Relates to #195 and #196

Directly implements a configuration to allow overriding the enterpriseUI database user and providing separation for access to the database by Engine/UI to aid in traceability/logging/auditing.

Indirectly addresses some standardization of DB host across resource files.

Willing to consider another templating mechanism outside of the ternary - it's self-documenting but not syntactically pretty.

@Btodhunter
Copy link
Member

Thanks for this PR @brandtkeller, it seems super useful! I need to do some manual testing on this change before merging it, but probably won't be able to get to that testing until sometime next week.

@flickerfly
Copy link
Contributor

@Btodhunter Is there any support that can be given to help get this over the hump? We're currently manually managing the secret to get this same result. Having a UI specific db user is working fine, but we haven't been able to trim down the level of permissions that may be required for that specific user yet.

@@ -15,9 +15,9 @@ metadata:
type: Opaque
stringData:
{{- if .Values.anchoreGlobal.dbConfig.ssl }}
ANCHORE_APPDB_URI: 'postgresql://{{ index .Values "postgresql" "postgresUser" }}:{{ index .Values "postgresql" "postgresPassword" }}@{{ template "db-hostname" . }}/{{ index .Values "postgresql" "postgresDatabase" }}?ssl=verify-full'
ANCHORE_APPDB_URI: 'postgresql://{{ ternary (index .Values "anchoreEnterpriseUi" "dbUser") (index .Values "postgresql" "postgresUser") (hasKey .Values.anchoreEnterpriseUi "dbUser" ) }}:{{ index .Values "postgresql" "postgresPassword" }}@{{ template "db-hostname" . }}/{{ index .Values "postgresql" "postgresDatabase" }}?ssl=verify-full'
Copy link
Member

Choose a reason for hiding this comment

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

@brandtkeller if .Values.anchoreEnterpriseUi.dbUser is set, should we be handling the password separately as well? Using .Values.postgresql.postgresPassword, which is shared with the engine db user, doesn't seem ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that is a fair assessment. We're trying to establish some ability to separate the users and their credentials - it would make sense to allow a non-shared password.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added support for both user/password to be unique. It's a lengthy template with multiple ternaries - so i'm open to input or anyone doing direct modification.

@Btodhunter
Copy link
Member

@brandtkeller I'll keep my eye on this and get it through asap.

@Btodhunter
Copy link
Member

@brandtkeller thanks for the update! I'm reviewing the template now. Can you bump the chart version? Then I'll merge this after finishing the review.

@Btodhunter
Copy link
Member

@brandtkeller you may also need to rebase on main to get the latest set of required tests in your github config.

Copy link
Member

@Btodhunter Btodhunter left a comment

Choose a reason for hiding this comment

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

lgtm!

@Btodhunter Btodhunter merged commit f4c2780 into anchore:main Jan 7, 2023
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.

Allow Engine/UI Database User separation
3 participants