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

Ory hydra #280

Merged
merged 20 commits into from
Oct 7, 2024
Merged

Ory hydra #280

merged 20 commits into from
Oct 7, 2024

Conversation

Bdegraaf1234
Copy link
Member

@Bdegraaf1234 Bdegraaf1234 commented Jul 1, 2024

Description of the change

Add ory-hydra as a component

Benefits

We can move away from managementportal as a resource server

Possible drawbacks

Applicable issues

Additional information

This is a stub/WIP

Checklist

  • Make sure application versions in compatiblity section of README is same as the versions tested in CI

@Bdegraaf1234 Bdegraaf1234 self-assigned this Jul 1, 2024
@Bdegraaf1234 Bdegraaf1234 marked this pull request as draft July 1, 2024 13:40
@Bdegraaf1234 Bdegraaf1234 changed the base branch from main to dev July 1, 2024 13:55
@Bdegraaf1234 Bdegraaf1234 force-pushed the ory-hydra branch 3 times, most recently from 1769208 to 425834b Compare July 8, 2024 13:23
@Bdegraaf1234 Bdegraaf1234 force-pushed the ory-hydra branch 2 times, most recently from 2d6e8eb to 30e7467 Compare July 12, 2024 09:12
@Bdegraaf1234 Bdegraaf1234 force-pushed the ory-hydra branch 3 times, most recently from adc0223 to 4032404 Compare July 15, 2024 19:56
@yatharthranjan
Copy link
Member

@pvannierop are there any plans on progressing this from the hyve?

@yatharthranjan
Copy link
Member

@pvannierop never mind, I am starting work on this now. Thanks.

@yatharthranjan
Copy link
Member

@mpgxvii can you please add the webhook jsonnet body and the updated identities please?

@mpgxvii
Copy link
Member

mpgxvii commented Sep 24, 2024

@mpgxvii can you please add the webhook jsonnet body and the updated identities please?

Added the identities and added the webhook jsonnet to the configmap here: RADAR-base/radar-helm-charts#260.

@yatharthranjan yatharthranjan marked this pull request as ready for review September 25, 2024 15:52
Copy link
Member

@yatharthranjan yatharthranjan left a comment

Choose a reason for hiding this comment

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

LGTM, if someone else can review too, then I will proceed with the merge

etc/hydra/values.yaml Outdated Show resolved Hide resolved
etc/hydra/values.yaml Outdated Show resolved Hide resolved
Copy link

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

LGTM!

@mpgxvii
Copy link
Member

mpgxvii commented Oct 4, 2024

Waiting on this release: RADAR-base/radar-helm-charts#262, then will merge.

@mpgxvii mpgxvii merged commit 6ffd19d into dev Oct 7, 2024
2 of 3 checks passed
@mpgxvii mpgxvii deleted the ory-hydra branch October 7, 2024 12:28
@keyvaann
Copy link
Collaborator

keyvaann commented Oct 7, 2024

@mpgxvii I tried to update our instance with changes from this PR and I get these messages:

-> kubectl logs kratos-selfservice-ui-node-radar-self-enrolment-ui-7f948f7qh4j4                                

> @ory/[email protected] start
> next start -p 3000

npm ERR! code EACCES
npm ERR! syscall mkdir
npm ERR! path /.npm
npm ERR! errno -13
npm ERR! 
npm ERR! Your cache folder contains root-owned files, due to a bug in
npm ERR! previous versions of npm which has since been addressed.
npm ERR! 
npm ERR! To permanently fix this problem, please run:
npm ERR!   sudo chown -R 10000:10000 "/.npm"

npm ERR! Log files were not written due to an error writing to the directory: /.npm/_logs
npm ERR! You can rerun the command with `--loglevel=verbose` to see the logs in your terminal

Also Helmfile sends this warning:

  W1007 19:14:41.326707 2290828 warnings.go:70] spec.template.spec.containers[0].env[10]: hides previous definition of "HYDRA_ADMIN_URL", which may be dropped when using apply

@mpgxvii
Copy link
Member

mpgxvii commented Oct 8, 2024

Hi @keyvaann, this is fixed, but it wasn't merged to the dev branch which is the docker image used here. I have now merged this and the docker image is now published.

@keyvaann
Copy link
Collaborator

keyvaann commented Oct 8, 2024

Hi @mpgxvii, how should I apply the fix?
I tried again to install the changes from the pr on our server and it failed again.

@mpgxvii
Copy link
Member

mpgxvii commented Oct 8, 2024

Hi @mpgxvii, how should I apply the fix? I tried again to install the changes from the pr on our server and it failed again.

@keyvaann Oh the permission issue is now fixed but I think the healthcheck need to be temporarily disabled since those changes are not yet merged. I'll open a PR for the updated values.

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.

5 participants