Skip to content

Conversation

@nurtai325
Copy link

No description provided.

@nurtai325 nurtai325 requested a review from lis0x90 as a code owner August 22, 2025 12:58
@github-actions
Copy link

github-actions bot commented Aug 22, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@nurtai325
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@lis0x90 lis0x90 requested a review from Smet2133 October 3, 2025 07:24
@nurtai325 nurtai325 marked this pull request as draft October 3, 2025 10:05
@github-actions github-actions bot added the enhancement New feature or request label Oct 14, 2025
@nurtai325 nurtai325 marked this pull request as ready for review October 14, 2025 15:01
@Netcracker Netcracker deleted a comment from sonarqubecloud bot Oct 14, 2025
@nurtai325 nurtai325 force-pushed the feature/k8s-token-authentication branch from da0c408 to 6f92c54 Compare October 15, 2025 06:15
@sonarqubecloud
Copy link

http: 0.0.0.0:5556

telemetry:
http: 0.0.0.0:5558
Copy link
Contributor

Choose a reason for hiding this comment

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

is this port used?

http: 0.0.0.0:5558

grpc:
addr: 0.0.0.0:5557
Copy link
Contributor

Choose a reason for hiding this comment

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

is this port used?

issuer: http://oidc-server:5556/dex

storage:
type: sqlite3
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need sqlite3 for tests?

grpc:
addr: 0.0.0.0:5557

connectors:
Copy link
Contributor

Choose a reason for hiding this comment

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

what do we mock?

)
.withNetwork(TEST_NETWORK)
.withExposedPorts(8080)
.withCopyFileToContainer(MountableFile.forHostPath(oidcTokenTempFile.toAbsolutePath()), "/var/run/secrets/kubernetes.io/serviceaccount/token")
Copy link
Contributor

Choose a reason for hiding this comment

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

where are the actual tests that use new auth ?

Copy link
Author

Choose a reason for hiding this comment

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

this is added because when application starts it looks for token and makes OIDC discovery. and this is added to prevent app from crashing at start in tests

account = serviceAccount
default:
if slices.Contains(roles, model.AnonymousRole) {
log.WarnC(userCtx, "Anonymous access will be dropped in future releases for: %s", ctx.OriginalURL())
Copy link
Contributor

Choose a reason for hiding this comment

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

code duplication. just check if the role == AnonymousRole in the beginning and skip other steps


log.InfoC(ctx, "Received request on create one more manage account")
if _, err := a.service.IsAccessGranted(ctx, username, password, model.ManagerAccountNamespace, []model.RoleName{model.ManagerRole}); err != nil {
if _, err := a.service.IsAccessGrantedWithBasic(ctx, username, password, model.ManagerAccountNamespace, []model.RoleName{model.ManagerRole}); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

so we have no option to create manager accounts with token? how can we deprecate the basic auth approach then?

Copy link
Contributor

Choose a reason for hiding this comment

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

what about client account for deployer v3?

Copy link
Author

Choose a reason for hiding this comment

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

it is not decided yet. it will be decided when maas becomes an operator. see stage 3 here. https://bass.netcracker.com/display/CPSEC/M2M+MaaS+Design

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants