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

fix: CE-1078 persist secret content changes between rollouts #698

Merged
merged 18 commits into from
Oct 17, 2024
Merged

fix: CE-1078 persist secret content changes between rollouts #698

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
935f1be
test commit
jon-funk Oct 8, 2024
95cc145
test commit
jon-funk Oct 8, 2024
249ef96
Add idir officer name update
jon-funk Oct 8, 2024
3ab9cdc
Add replica config for test namespace workloads
jon-funk Oct 8, 2024
9f60ca7
rollingupdates for test replicas
jon-funk Oct 8, 2024
8d8795f
update chart lock
jon-funk Oct 8, 2024
50e1ae9
annotation sanity check
jon-funk Oct 8, 2024
850c72a
test commit
jon-funk Oct 8, 2024
d304f7d
Revert annotation check and allow secrets to persist in PR dev infra
jon-funk Oct 8, 2024
87eb7f9
Revert replica tests
jon-funk Oct 8, 2024
41feeb1
test commit
jon-funk Oct 8, 2024
996f87e
Add labels for cleanup of PR related releases
jon-funk Oct 11, 2024
762d121
Add additional where to update
jon-funk Oct 11, 2024
b3b3aba
formatting tidy
jon-funk Oct 11, 2024
a49fdf8
trigger pr action
jon-funk Oct 11, 2024
012e984
Cleanup space
jon-funk Oct 11, 2024
8d512a7
Merge branch 'release/lions-mane-jellyfish' into CE-1078
afwilcox Oct 16, 2024
38f1425
Merge branch 'release/lions-mane-jellyfish' into CE-1078
afwilcox Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/pr-close.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,12 @@ jobs:
with:
cleanup: helm
packages: backend frontend migrations

cleanup-labeled:
name: Cleanup Labeled Resources
uses: bcgov/quickstart-openshift-helpers/.github/workflows/[email protected]
secrets:
oc_namespace: ${{ secrets.OC_NAMESPACE }}
oc_token: ${{ secrets.OC_TOKEN }}
with:
cleanup: label
1 change: 0 additions & 1 deletion .github/workflows/pr-open.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ jobs:
oc_token: ${{ secrets.OC_TOKEN }}
with:
triggers: ('backend/' 'frontend/' 'webeoc/' 'migrations/')
params: --set global.secrets.persist=false

tests:
name: Tests
Expand Down
7 changes: 5 additions & 2 deletions charts/app/Chart.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,8 @@ dependencies:
- name: postgresql
repository: https://charts.bitnami.com/bitnami
version: 15.5.13
digest: sha256:e29db8b50c7ad4e611c43b4590506d2172ac278b08f015fe0240ba123f2166ec
generated: "2024-07-03T12:48:59.269045601Z"
- name: nats
repository: https://nats-io.github.io/k8s/helm/charts/
version: 1.1.12
digest: sha256:daadb6fa80ea04bc755dfeae1b823ce70b60f7bc3326ab9c40b3768bb2d5d9aa
generated: "2024-10-08T14:08:20.367635823-07:00"
1 change: 1 addition & 0 deletions charts/app/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,6 @@ Selector labels
{{- define "selectorLabels" -}}
app.kubernetes.io/name: {{ include "fullname" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app: {{ .Release.Name }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need this new label?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's so that this label variant of the cleanup action can be re-used here

nr-compliance-enforcement/.github/workflows/pr-close.yml

Line 23 in 762d121

cleanup-labeled:

As the helm pr-close here https://github.com/bcgov/quickstart-openshift-helpers/blob/main/.github/workflows/.pr-close.yml#L161 does not allow flag passthrough. Specificaly in this case global.secrets.persist=false as the dev team would like secret workloads to persist in their dev environments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changes to their secret workloads to persist between deployments*

Copy link
Collaborator

@mishraomp mishraomp Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

lets discuss this, as clean up with label will make helm to stick around with secret objects, @DerekRoberts and I can look at the helper function to add in necessary flags as needed or we can add you as a collaborator on those repos :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am sorry.
I misunderstood the change, I see now, that you are cleaning up with helm then with label to make sure the persisted secret is deleted with labels, as helm wont delete them during uninstall

I will keep that into our helper as a backlog item, so we don't do twice cleanups :)

Copy link
Member

Choose a reason for hiding this comment

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

@jon-funk I would recommend strongly against persisting DEV secrets, since they tend to be abused. Keep them GitHub Actions (or even, yuk, Vault), then populate during PR environment creation.

Copy link
Collaborator

@afwilcox afwilcox Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@DerekRoberts @mishraomp - Let me provide a little context as to why we want this change.

Our project has two repositories. This one, and the nr-compliance-enforcement-cm repository that has the case management APIs and data that live in the emerald cluster.

By default the PRs in this repository are pointing to a static version of dev, in our emerald namespace.

However in the event of PRs that require changes both here and for the other repository we will end up needing to overwrite that secret to point to the environment where the other PR is deployed (e.g. dev-4).

The problem is... EVERY single commit on this repository (e.g. bringing PR up to date with the latest changes) will reset that overridden secret back to dev-static, which will usually result in integration tests failing.

As a result whenever changes are made to a PR under review one needs to

  • Commit Changes
  • Revert secret from default value back to overridden value
  • Ensure that initial helm deployment has finished
  • Nudge OpenShift pods so that proper secret is picked up
  • Restart integration tests if they were failing because earlier steps weren't done in time

Can you provide any suggestions for how we can streamline this that doesn't involve persisting secrets?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ent of PRs that require changes both here and for the other repository we will end up needing to overwrite that secret to point to the environment where the other PR is deployed (e.g. dev-4).

lets have a quick discussion on this one over teams, we can have the .deployer.yml in the repo itself and pass in github secrets, which would allow us github secrets as the source of truth , so we can keep a human generated secret in both repos, which can be fed to the pipeline and we don't need to persist secrets across prs or rollouts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jon-funk , lets update the logic here to look for pr specific secret objects, if they exist, read from them, if not read from default, https://github.com/bcgov/nr-compliance-enforcement/blob/main/charts/app/templates/secret.yaml#L5

Happy to have a chat on this over teams

Copy link
Member

Choose a reason for hiding this comment

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

@afwilcox @jon-funk How about reading secrets from the second repo and synching them over? It'd eliminate a lot of busy work. There are tons of alternatives too, like just plain copying the remote repo's config to GitHub secrets to easily (re-)population. @mishraomp's chat idea works too, since this isn't as easy as we usually see. ;)

{{- end }}

2 changes: 2 additions & 0 deletions charts/app/templates/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
{{- $caseManagementApiUrl := (get $secretData "caseManagementApiUrl" | b64dec | default "") }}




---
apiVersion: v1
kind: Secret
Expand Down
10 changes: 10 additions & 0 deletions migrations/migrations/R__Create-Test-Data.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,16 @@ VALUES
'f896cbb2d5254e54a4ad581dc80681d1'::uuid
) ON CONFLICT
DO NOTHING;
-- Fix keycloak name
UPDATE
public.officer
SET
user_id = 'JONFUNK'
WHERE
(
officer_guid = 'b17ee2c1-a26b-4911-ac6f-810b8fdfaab3'
AND user_id = 'JFUNK'
);

INSERT INTO
public.officer (
Expand Down
2 changes: 2 additions & 0 deletions migrations/test-only-migrations/R__Test-Data-Creation.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ INSERT INTO public.officer
(officer_guid, user_id, create_user_id, create_utc_timestamp, update_user_id, update_utc_timestamp, person_guid, office_guid, auth_user_guid)
VALUES('b17ee2c1-a26b-4911-ac6f-810b8fdfaab3'::uuid, 'JFUNK', 'FLYWAY', '2024-01-22 22:16:16.754', 'FLYWAY', '2024-01-22 22:20:48.186', '97f3cee5-6f4a-410f-810f-d431595fccee'::uuid, '4a5a94b1-bd47-4611-a577-861d97089903'::uuid, 'f896cbb2d5254e54a4ad581dc80681d1'::uuid)
ON CONFLICT DO NOTHING;
-- Fix keycloak name
UPDATE public.officer SET user_id = 'JONFUNK' WHERE officer_guid = 'b17ee2c1-a26b-4911-ac6f-810b8fdfaab3';
INSERT INTO public.officer
(officer_guid, user_id, create_user_id, create_utc_timestamp, update_user_id, update_utc_timestamp, person_guid, office_guid, auth_user_guid)
VALUES('06ff894b-3895-4d32-8a4a-1fcc0be23e47'::uuid, 'RRONDEAU', 'FLYWAY', '2024-01-22 22:16:16.754', 'FLYWAY', '2024-01-22 22:20:48.186', '0667495f-61a5-4d3b-b756-1ee58cb38e23'::uuid, '4a5a94b1-bd47-4611-a577-861d97089903'::uuid, '77c6040d69b74757903f1cba37404db4'::uuid)
Expand Down
Loading