Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

THREESCALE-11441 only validate oidc setting if authentication method is set to oidc #1568

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

Open
tkan145 wants to merge 1 commit into 3scale:master
base: master
Choose a base branch
Loading
from tkan145:THREESCALE-11441

Conversation

@tkan145
Copy link
Contributor

@tkan145 tkan145 commented Jan 30, 2026 •
edited
Loading

What

Fix https://issues.redhat.com/browse/THREESCALE-11441

Verification steps

  • Checkout this branch
  • Build APIcast image
make runtime-image IMAGE_NAME=apicast-test
  • Get into dev-environment
cd dev-environments/plain-http-upstream
  • Modify apicast-config.json as follow
diff --git a/dev-environments/plain-http-upstream/apicast-config.json b/dev-environments/plain-http-upstream/apicast-config.json
index ff944273..405498c6 100644 
--- a/dev-environments/plain-http-upstream/apicast-config.json 
+++ b/dev-environments/plain-http-upstream/apicast-config.json 
@@ -5,6 +5,7 @@ 
 "backend_version": "1", 
 "proxy": { 
 "hosts": ["get.example.com"], 
+ "oidc_issuer_endpoint": "https://3scale-test:foo@rhsso.test/auth/realms/3scale", 
+ "authentication_method": "1", 
 "api_backend": "http://example.com/get", 
 "backend": { 
 "endpoint": "http://backend:80",

and docker-compose

diff --git a/dev-environments/plain-http-upstream/docker-compose.yml b/dev-environments/plain-http-upstream/docker-compose.yml
index ebf84ebc..0563effa 100644 
--- a/dev-environments/plain-http-upstream/docker-compose.yml 
+++ b/dev-environments/plain-http-upstream/docker-compose.yml 
@@ -10,11 +10,11 @@ services: 
 - two.upstream 
 environment: 
 THREESCALE_CONFIG_FILE: /tmp/config.json 
- THREESCALE_DEPLOYMENT_ENV: staging 
- APICAST_CONFIGURATION_LOADER: lazy 
+ THREESCALE_DEPLOYMENT_ENV: production 
+ APICAST_CONFIGURATION_LOADER: boot 
 APICAST_WORKERS: 1 
 APICAST_LOG_LEVEL: debug 
- APICAST_CONFIGURATION_CACHE: "0" 
+ APICAST_CONFIGURATION_CACHE: "300" 
 expose: 
 - "8080" 
 - "8090"
  • Start APIcast with latest image first,
make gateway IMAGE_NAME=quay.io/3scale/apicast:latest

the gateway should output the following

gateway-1 | 2026年01月30日 00:13:53 [debug] 23#23: *2 [lua] http.lua:86: connect(): connected to ip:172.19.0.2 host: keycloak port: 8080 ok: nil err: connection refused 
gateway-1 | 2026年01月30日 00:13:53 [error] 23#23: *2 [lua] discovery.lua:113: openid_configuration(): failed to get OIDC Provider from http://oidc-issuer-for-3scale:oidc-issuer-for-3scale-secret@keycloak:8080/re
alms/basic/.well-known/openid-configuration status: 0 body: nil, context: ngx.timer 
gateway-1 | 2026年01月30日 00:13:53 [notice] 23#23: gracefully shutting down 
gateway-1 | 2026年01月30日 00:13:53 [notice] 23#23: exiting
  • Stop the gateway
CTRL-C
  • Start the gateway again with new image
make gateway IMAGE_NAME=apicast-test

Check the log again and you should not see the same error

@tkan145 tkan145 requested a review from a team as a code owner January 30, 2026 00:22
@tkan145 tkan145 changed the title (削除) fix: only validate oidc setting if authentication method is set to oidc (削除ここまで) (追記) THREESCALE-11441 only validate oidc setting if authentication method is set to oidc (追記ここまで) Jan 30, 2026
@tkan145 tkan145 force-pushed the THREESCALE-11441 branch 2 times, most recently from 842cd5f to 2bd8459 Compare February 2, 2026 08:27
Copy link

@josemigallas josemigallas left a comment

Choose a reason for hiding this comment

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

LGTM with some minor suggestions

}, config.oidc)
end)

it('ingore OIDC configuration when authentication_method is not oidc', function()
Copy link

@josemigallas josemigallas Feb 3, 2026

Choose a reason for hiding this comment

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

Suggested change
it('ingore OIDC configuration when authentication_method is not oidc', function()
it('ignore OIDC configuration when authentication_method is not oidc', function()

tkan145 reacted with thumbs up emoji
Comment on lines +26 to +27
-- Only fetch OIDC configuration if authentication method is set to 'oidc'
local authentication = proxy.authentication_method or service.backend_version
Copy link

@josemigallas josemigallas Feb 3, 2026

Choose a reason for hiding this comment

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

I think this comment could go away by simply extracting a guard from the code below:

Suggested change
-- Only fetch OIDC configuration if authentication method is set to 'oidc'
local authentication = proxy.authentication_method or service.backend_version
local authentication = proxy.authentication_method or service.backend_version
if authentication ~= 'oidc' then
return nil
end

The if authentication part seems redundant anyway (nil is already unequal to 'oidc').

Copy link
Contributor Author

@tkan145 tkan145 Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The authentication can take the following values:

  • 1 : user_key
  • 2: app_id/app_key
  • oidc: OIDC
  • nil: But if the oidc_issuer_endpoint is provided, then in this case we don't want to query OIDC endpoint
    Therefore, an if statement is needed.

local authentication = proxy.authentication_method or service.backend_version

if authentication and authentication == 'oidc' then
local result = _M.discovery:call(service.proxy.oidc_issuer_endpoint)
Copy link

@josemigallas josemigallas Feb 3, 2026

Choose a reason for hiding this comment

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

proxy already declared on the top

Suggested change
local result = _M.discovery:call(service.proxy.oidc_issuer_endpoint)
local result = _M.discovery:call(proxy.oidc_issuer_endpoint)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josemigallas josemigallas josemigallas left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /