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

chore: bump dependencies #999

Merged
merged 18 commits into from Sep 8, 2022
Merged

chore: bump dependencies #999

merged 18 commits into from Sep 8, 2022

Conversation

hperl
Copy link
Contributor

@hperl hperl commented Sep 1, 2022

This PR swaps the configuration library from viperx for koanf and updates the dependencies, most importantly to ory/x.

This work relates to the effort to use Oathkeeper as a gRPC/HTTP middleware in other projects.

Fixes #757

Breaking changes

  • The health APIs now return a structured error if the service is not available as a JSON with the fields status, code , and message
  • The environment variable JAEGER_PROPAGATION was removed. Use TRACING_PROVIDER_JAEGER_PROPAGATION

@hperl hperl requested a review from aeneasr as a code owner September 1, 2022 10:51
@hperl hperl requested a review from zepatrik September 1, 2022 10:51
@hperl hperl changed the title Chore: Bump Dependencies chore: Bump Dependencies Sep 1, 2022
@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #999 (121e3c5) into master (972f37f) will increase coverage by 11.30%.
The diff coverage is 83.91%.

❗ Current head 121e3c5 differs from pull request most recent head 78c0acb. Consider uploading reports for the commit 78c0acb to get more accurate results

@@             Coverage Diff             @@
##           master     #999       +/-   ##
===========================================
+ Coverage   66.49%   77.79%   +11.30%     
===========================================
  Files         107       81       -26     
  Lines        4796     3873      -923     
===========================================
- Hits         3189     3013      -176     
+ Misses       1325      587      -738     
+ Partials      282      273        -9     
Impacted Files Coverage Δ
api/credential.go 75.60% <ø> (ø)
api/decision.go 95.55% <ø> (ø)
api/health.go 0.00% <ø> (ø)
api/rule.go 52.17% <ø> (ø)
cmd/health.go 50.00% <0.00%> (ø)
cmd/root.go 15.38% <22.22%> (-30.07%) ⬇️
driver/driver_default.go 81.81% <60.00%> (-18.19%) ⬇️
x/testhelper.go 62.50% <62.50%> (ø)
internal/driver.go 77.77% <75.00%> (-22.23%) ⬇️
rule/fetcher_default.go 78.57% <78.78%> (+1.91%) ⬆️
... and 50 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@hperl hperl changed the title chore: Bump Dependencies chore: bump dependencies Sep 1, 2022
.schema/config.schema.json Outdated Show resolved Hide resolved
cmd/serve.go Show resolved Hide resolved
codecov.yml Show resolved Hide resolved
credentials/fetcher_default.go Outdated Show resolved Hide resolved
driver/configuration/provider_koanf.go Show resolved Hide resolved
pipeline/authz/remote_json.go Show resolved Hide resolved
pipeline/authz/remote_json.go Outdated Show resolved Hide resolved
rule/fetcher_default_test.go Outdated Show resolved Hide resolved
rule/fetcher_default_test.go Show resolved Hide resolved
spec/api.json Outdated Show resolved Hide resolved
@hperl hperl self-assigned this Sep 1, 2022
Copy link
Member

@zepatrik zepatrik left a comment

Choose a reason for hiding this comment

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

This looks very good, just found some minor things 🎉

driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
driver/configuration/provider_koanf.go Show resolved Hide resolved
driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
pipeline/mutate/mutator_hydrator.go Outdated Show resolved Hide resolved
proxy/request_handler_test.go Show resolved Hide resolved
rule/fetcher_default_test.go Outdated Show resolved Hide resolved
rule/fetcher_default_test.go Show resolved Hide resolved
spec/api.json Outdated Show resolved Hide resolved
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

First-pass review :)

.schema/embed.go Show resolved Hide resolved
api/credential_test.go Outdated Show resolved Hide resolved
codecov.yml Show resolved Hide resolved
.schema/config.schema.json Outdated Show resolved Hide resolved
cmd/root.go Show resolved Hide resolved
cmd/serve.go Show resolved Hide resolved
credentials/fetcher_default.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@hperl hperl force-pushed the chore/bump-deps branch 3 times, most recently from 39920db to 5ef9120 Compare September 4, 2022 15:52
@hperl hperl force-pushed the chore/bump-deps branch 3 times, most recently from 25c5629 to a998323 Compare September 5, 2022 12:33
zepatrik
zepatrik previously approved these changes Sep 6, 2022
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Great! This is basically in a mergable state with a few questions and requirements:

  • in health_test.go it looks as if the health payloads have changed. If so, we should add this as a breaking change. Can you please update the PR description to match the changes made?
  • some configuration environment variables have been deprecated, please add those to the breaking changes list above.
  • check that the removed env vars are not used in helm charts and in ory cloud
  • Search the documentation for removed config keys and remove them

driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
driver/configuration/provider_koanf.go Outdated Show resolved Hide resolved
pipeline/authz/keto_engine_acp_ory.go Outdated Show resolved Hide resolved
rule/fetcher_default.go Show resolved Hide resolved
rule/fetcher_default.go Show resolved Hide resolved
rule/fetcher_default_test.go Outdated Show resolved Hide resolved
rule/fetcher_default_test.go Show resolved Hide resolved
@aeneasr aeneasr mentioned this pull request Sep 7, 2022
7 tasks
hperl and others added 10 commits September 7, 2022 14:49
* bump ory/x deps
* update to new httpx, metricsx
* cleanup viper config
* revert schema
* bump swagger
* remove extra sets
* simplify CORS settings getter
* move to resilitent httpx completely
Co-authored-by: Patrik <zepatrik@users.noreply.github.com>
@hperl
Copy link
Contributor Author

hperl commented Sep 7, 2022

Great! This is basically in a mergable state with a few questions and requirements:

  • in health_test.go it looks as if the health payloads have changed. If so, we should add this as a breaking change. Can you please update the PR description to match the changes made?
  • some configuration environment variables have been deprecated, please add those to the breaking changes list above.
  • check that the removed env vars are not used in helm charts and in ory cloud
  • Search the documentation for removed config keys and remove them

Done, no references to JAEGER_PROPAGATION were found in docs, cloud, k8s.

@hperl hperl force-pushed the chore/bump-deps branch 2 times, most recently from 121e3c5 to f33dce4 Compare September 8, 2022 09:43
* remove broken caching for e2e tests
* re-add `--config` flag
@aeneasr aeneasr merged commit 6bac536 into master Sep 8, 2022
@aeneasr aeneasr deleted the chore/bump-deps branch September 8, 2022 14:52
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.

Adopt new config pipeline
3 participants