Add new security defn for abi

[pawanpinjarkar]

List all the issues related to this PR

• [ ] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [x] No tests needed

Checklist

• [ ] Title and description added to both, commit and PR.
• [ ] Relevant issues have been associated (see CONTRIBUTING guide)
• [ ] This change does not require a documentation update (docstring, docs, README, etc)
• [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

[openshift-ci[bot]]

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

[openshift-ci-robot]

@pawanpinjarkar: This pull request references AGENT-951 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

The agent-based installer needs a new security definition to perform the read-only operations for the user persona watcher. The agent-based installer commands such as wait-for and monitor internally uses the read-only persona watcher which can only make read requests to the endpoints annotated with watcherAuth security definition.

Other existing authenticators do not support this agent-based installer specific watcherAuth security definition.

List all the issues related to this PR

• [x] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [x] No tests needed

Checklist

• [ ] Title and description added to both, commit and PR.
• [ ] Relevant issues have been associated (see CONTRIBUTING guide)
• [ ] This change does not require a documentation update (docstring, docs, README, etc)
• [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[pawanpinjarkar]

/cc @carbonin

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 29.31034% with 41 lines in your changes missing coverage. Please review.

Project coverage is 68.79%. Comparing base (e971390) to head (bfd297f). Report is 16 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/auth/agent_local_authz_handler.go	0.00%	34 Missing :warning:
pkg/auth/none_authenticator.go	0.00%	3 Missing :warning:
pkg/auth/authz.go	77.77%	2 Missing :warning:
pkg/auth/rhsso_authenticator.go	0.00%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6784      +/-   ##
==========================================
- Coverage   68.81%   68.79%   -0.03%     
==========================================
  Files         251      272      +21     
  Lines       37630    40009    +2379     
==========================================
+ Hits        25895    27523    +1628     
- Misses       9417    10072     +655     
- Partials     2318     2414      +96     

Files with missing lines	Coverage Δ
pkg/auth/agent_local_authenticator.go	56.86% <100.00%> (+3.67%)	:arrow_up:
pkg/auth/authenticator.go	100.00% <ø> (ø)
pkg/auth/local_authenticator.go	50.64% <100.00%> (+1.31%)	:arrow_up:
pkg/auth/authz.go	85.71% <77.77%> (-14.29%)	:arrow_down:
pkg/auth/rhsso_authenticator.go	50.90% <0.00%> (-0.63%)	:arrow_down:
pkg/auth/none_authenticator.go	6.89% <0.00%> (-0.80%)	:arrow_down:
pkg/auth/agent_local_authz_handler.go	0.00% <0.00%> (ø)

... and 45 files with indirect coverage changes

[pawanpinjarkar]

/test edge-subsystem-kubeapi-aws

[pawanpinjarkar]

/test edge-subsystem-kubeapi-aws

[carbonin]

So this looks okay, but is it really what you want to do?

As far as I can tell this just adds another way to provide the same token, so you're not really adding any additional security because someone with that token could provide it using a different header and still use other endpoints.

For this to actually achieve authorization you'd need to issue separate tokens that have some kind of claim that identifies them as read-only. Then in the token verification process somewhere ensure that a read-only token is only valid for the endpoints it's assigned to.

[pawanpinjarkar]

So this looks okay, but is it really what you want to do?

As far as I can tell this just adds another way to provide the same token, so you're not really adding any additional security because someone with that token could provide it using a different header and still use other endpoints.

For this to actually achieve authorization you'd need to issue separate tokens that have some kind of claim that identifies them as read-only. Then in the token verification process somewhere ensure that a read-only token is only valid for the endpoints it's assigned to.

yes, so my plan is to have the swagger changes from this PR. Then in a different PR, implement a new authz handler in assisted service for ABI and update the installer to create 3 seperate tokens. Code changes from all the 3 PRs then will do the intended operation.

[carbonin]

I think it would be fine to add the authz stuff in this same PR. It gives context that makes these changes worthwhile.

Also I wouldn't want to have to go back and change the swagger again if we run into issues later with the next patch.

[openshift-ci-robot]

@pawanpinjarkar: This pull request references AGENT-949 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

This pull request references AGENT-951 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

The agent-based installer needs a new security definition to perform the read-only operations for the user persona watcher. The agent-based installer commands such as wait-for and monitor internally uses the read-only persona watcher which can only make read requests to the endpoints annotated with watcherAuth security definition.

Other existing authenticators do not support this agent-based installer specific watcherAuth security definition.

List all the issues related to this PR

• [x] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [x] No tests needed

Checklist

• [ ] Title and description added to both, commit and PR.
• [ ] Relevant issues have been associated (see CONTRIBUTING guide)
• [ ] This change does not require a documentation update (docstring, docs, README, etc)
• [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@pawanpinjarkar: This pull request references AGENT-949 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

This pull request references AGENT-951 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

The agent-based installer needs a new security definition to perform the read-only operations for the user persona watcher. The agent-based installer commands such as wait-for and monitor internally uses the read-only persona watcher which can only make read requests to the endpoints annotated with watcherAuth security definition.

Other existing authenticators do not support this agent-based installer specific watcherAuth security definition.

• Read 3 seperate auth tokens - AGENT_AUTH_TOKEN, USER_AUTH_TOKEN and WATCHER_AUTH_TOKEN
• Implememnt a new authorization handler agent_local_authz_handler
• Implement new security definition AuthWatcherAuth

List all the issues related to this PR

• [x] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [x] No tests needed

Checklist

• [ ] Title and description added to both, commit and PR.
• [ ] Relevant issues have been associated (see CONTRIBUTING guide)
• [ ] This change does not require a documentation update (docstring, docs, README, etc)
• [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@pawanpinjarkar: This pull request references AGENT-949 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

This pull request references AGENT-951 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

The agent-based installer needs a new security definition to perform the read-only operations for the user persona watcher. The agent-based installer commands such as wait-for and monitor internally use the read-only persona watcher which can only make read requests to the endpoints annotated with watcherAuth security definition.

Other existing authenticators do not support this agent-based installer specific watcherAuth security definition.

• Read and set USER_AUTH_TOKEN
• Implememnt a new authorization handler agent_local_authz_handler
• Implement new security definition AuthWatcherAuth

List all the issues related to this PR

• [x] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [ ] dev-scripts environment
• [ ] Reviewer's test appreciated
• [ ] Waiting for CI to do a full test run
• [ ] Manual (Elaborate on how it was tested)
• [x] No tests needed

Checklist

• [ ] Title and description added to both, commit and PR.
• [ ] Relevant issues have been associated (see CONTRIBUTING guide)
• [ ] This change does not require a documentation update (docstring, docs, README, etc)
• [ ] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@pawanpinjarkar: This pull request references AGENT-949 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

This pull request references AGENT-951 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

The agent-based installer needs a new security definition to perform the read-only operations for the user persona watcher. The agent-based installer commands such as wait-for and monitor internally use the read-only persona watcher which can only make read requests to the endpoints annotated with watcherAuth security definition.

Other existing authenticators do not support this agent-based installer specific watcherAuth security definition.

• Read and set USER_AUTH_TOKEN
• Implememnt a new authorization handler agent_local_authz_handler
• Implement new security definition AuthWatcherAuth

List all the issues related to this PR

• [x] New Feature
• [ ] Enhancement
• [ ] Bug fix
• [ ] Tests
• [ ] Documentation
• [ ] CI/CD

What environments does this code impact?

• [ ] Automation (CI, tools, etc)
• [ ] Cloud
• [ ] Operator Managed Deployments
• [x] None

How was this code tested?

• [ ] assisted-test-infra environment
• [x] dev-scripts environment
• [ ] Reviewer's test appreciated
• [x] Waiting for CI to do a full test run
• [x] Manual (Elaborate on how it was tested) Tested locally with dev-scripts using the installer PR https://github.com/openshift/installer/pull/9039
• [x] No tests needed

Checklist

• [x] Title and description added to both, commit and PR.
• [x] Relevant issues have been associated (see CONTRIBUTING guide)
• [x] This change does not require a documentation update (docstring, docs, README, etc)
• [x] Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[pawanpinjarkar]

/test edge-e2e-metal-assisted-mtv-4-17

[pawanpinjarkar]

/hold need to make sure the installer PR https://github.com/openshift/installer/pull/9039 lands simultaneously.

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carbonin, pawanpinjarkar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [carbonin]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[carbonin]

/lgtm

[pawanpinjarkar]

/hold cancel

[pawanpinjarkar]

/override e2e-agent-compact-ipv4

[openshift-ci[bot]]

@pawanpinjarkar: pawanpinjarkar unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers.

In response to this:

/override e2e-agent-compact-ipv4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 7a08e5079a2c616c83e4ca41b23f433a7d38022b and 2 for PR HEAD bfd297fcf46f55542c9264e2604dca6d145e76bb in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 7a08e5079a2c616c83e4ca41b23f433a7d38022b and 2 for PR HEAD bfd297fcf46f55542c9264e2604dca6d145e76bb in total

[rwsu]

/test e2e-agent-compact-ipv4

Last failure appears to be infrastructure related:

Trying to pull quay.io/openshifttest/squid-proxy:multiarch...
Getting image source signatures
Copying blob sha256:c5fccdba25e8a523e654a2ee169250141a5741e8844dee063cbbbf0e5e2d4bb9
Copying blob sha256:970e121bff396815d64dc4e784589369b310fb6630281804ca684a7d34df7c63
Copying blob sha256:0d1baa9a1e65a2a5993ffaad35ff54cfb88cd20846309f34746d67cc421dbfe9
Copying blob sha256:3662bf1aea374e1e6919b47fac27665af0b8c1d86189ae43b5527c663ef79ce4
Copying blob sha256:07222971243684a01bce78451c72c5b314109024de2d66bf7c31386f6d68d72b
Copying blob sha256:70fb9965a23f2226fef622992fdf507b8333c61d68259766d4721cc4ba1e5dae
Copying blob sha256:dbb1afa921ac3695aad1cad01d9f2d53e2a983b8ee52aa34109463de403e4fcc
Copying config sha256:4f135017348a2539121a6476c0f8bc32b99ff8008f205a5eed5541a98d971989
Writing manifest to image destination
Error: OCI runtime error: crun: unknown version specified
{"component":"entrypoint","error":"wrapped process failed: exit status 126","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:84","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"2024-11-13T00:42:12Z"}
error: failed to execute wrapped command: exit status 126

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 086bd745d54e9c2b46aa7ffc8167af7d4cec3949 and 2 for PR HEAD bfd297fcf46f55542c9264e2604dca6d145e76bb in total

[pawanpinjarkar]

/test e2e-agent-compact-ipv4

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD d78ad548738d1dd0db827dc8d06447b3d7f5443e and 2 for PR HEAD bfd297fcf46f55542c9264e2604dca6d145e76bb in total

[pawanpinjarkar]

/retest-required

[openshift-ci[bot]]

@pawanpinjarkar: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.