Skip to content

Central opa deployment#167

Merged
garryod merged 7 commits intomainfrom
central-opa-deployment
Nov 1, 2024
Merged

Central opa deployment#167
garryod merged 7 commits intomainfrom
central-opa-deployment

Conversation

@garryod
Copy link
Contributor

@garryod garryod commented Sep 16, 2024

Requires #164

@garryod garryod added enhancement New feature or request helm Pull request that updates Helm charts labels Sep 16, 2024
@garryod garryod self-assigned this Sep 16, 2024
@garryod garryod marked this pull request as draft September 16, 2024 15:01
@garryod garryod force-pushed the central-opa-deployment branch 12 times, most recently from ed98604 to ce168b0 Compare September 17, 2024 15:54
@garryod garryod marked this pull request as ready for review September 17, 2024 15:56
Copy link

@DiamondJoseph DiamondJoseph left a comment

Choose a reason for hiding this comment

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

Will discuss tomorrow

@garryod garryod force-pushed the central-opa-deployment branch 4 times, most recently from 3065ad4 to 10cf27b Compare November 1, 2024 10:42
@garryod garryod force-pushed the central-opa-deployment branch from 10cf27b to d9857ba Compare November 1, 2024 11:20
@tpoliaw
Copy link
Collaborator

tpoliaw commented Nov 1, 2024

I don't understand what's going on here well enough to properly review it but as this is currently what is being used to deploy the service I think it should be merged so that any changes in future can be tracked as separate changes.

tpoliaw
tpoliaw previously approved these changes Nov 1, 2024
MattPrit
MattPrit previously approved these changes Nov 1, 2024
Copy link
Collaborator

@MattPrit MattPrit left a comment

Choose a reason for hiding this comment

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

LGTM
(Perhaps the remaining references to CAS and userinfo environment variables should be removed separately?)

@garryod
Copy link
Contributor Author

garryod commented Nov 1, 2024

LGTM (Perhaps the remaining references to CAS and userinfo environment variables should be removed separately?)

Think this is the best PR to remove them. Though I forgot one instance in charts/apps/values,yaml so have also removed that now

@MattPrit
Copy link
Collaborator

MattPrit commented Nov 1, 2024

LGTM (Perhaps the remaining references to CAS and userinfo environment variables should be removed separately?)

Think this is the best PR to remove them. Though I forgot one instance in charts/apps/values,yaml so have also removed that now

There are also references in .devcontainer/docker-compose.yml as well as docs, are they best dealt with here as well?

@garryod
Copy link
Contributor Author

garryod commented Nov 1, 2024

There are also references in .devcontainer/docker-compose.yml as well as docs, are they best dealt with here as well?

Probably best in a separate PR... I'd forgotten I wrote so many docs 😢

Edit: see #199

@garryod garryod merged commit df3f174 into main Nov 1, 2024
@garryod garryod deleted the central-opa-deployment branch January 9, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request helm Pull request that updates Helm charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants