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

AGENT-875: Authenticate agents #8395

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pawanpinjarkar
Copy link
Contributor

@pawanpinjarkar pawanpinjarkar commented May 11, 2024

  • Set JWT token in the expected env var AGENT_AUTH_TOKEN
  • Set authorization header in the API requests

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 11, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 11, 2024

@pawanpinjarkar: This pull request references AGENT-875 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 task to target the "4.16.0" version, but no target version was set.

In response to this:

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 openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2024
Copy link
Contributor

openshift-ci bot commented May 11, 2024

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

@pawanpinjarkar
Copy link
Contributor Author

/hold

@pawanpinjarkar pawanpinjarkar marked this pull request as ready for review May 11, 2024 01:19
@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 11, 2024
Copy link
Contributor

openshift-ci bot commented May 11, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from pawanpinjarkar. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 13, 2024

@pawanpinjarkar: This pull request references AGENT-875 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 task to target the "4.16.0" version, but no target version was set.

In response to this:

  • Set JWT token in the expected env var PULL_SECRET_TOKEN
  • Set authorization header in the API requests

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
Copy link
Contributor Author

/hold cancel

@pawanpinjarkar
Copy link
Contributor Author

/cc @rwsu @andfasano

@openshift-ci openshift-ci bot requested review from andfasano and rwsu May 13, 2024 14:16
}

// Probably also need to add PULL_SECRET_TOKEN for authentication.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rwsu WDYT? I think the token will be needed for the nodes making API requests to assisted service once the new auth type is enabled.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's required here because both services already include rendezvous-host.env where you already added it above.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2024
@@ -411,10 +411,12 @@ func getRendezvousHostEnv(serviceProtocol, nodeZeroIP string, workflowType workf
return fmt.Sprintf(`NODE_ZERO_IP=%s
SERVICE_BASE_URL=%s
IMAGE_SERVICE_BASE_URL=%s
PULL_SECRET_TOKEN=%s
Copy link
Member

Choose a reason for hiding this comment

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

This currently results in the same token being used for both the agents and the scripts that drive the assisted API. I can't remember what we said about how we would do authz in the future, but maybe it makes sense to pass them separately even if they have the same value right now.
I guess that would also solve the issue that "PULL_SECRET_TOKEN" is not the ideal name from assisted-service's perspective, bit is required for assisted-installer-agent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, its been renamed to AGENT_AUTH_TOKEN

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 16, 2024

@pawanpinjarkar: This pull request references AGENT-875 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 task to target the "4.16.0" version, but no target version was set.

In response to this:

  • Set JWT token in the expected env var AGENT_AUTH_TOKEN
  • Set authorization header in the API requests

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
Copy link
Contributor Author

pawanpinjarkar commented May 16, 2024

To test the authentication feature, this and other code changes from relevant PRs need to work together.
Please note, this PR only adds the code necessary to authenticate agent service, however, the complete functionality to work end to end depends on below PRs

And most importantly, updating the auth type env var

Copy link
Contributor

openshift-ci bot commented Jun 7, 2024

@pawanpinjarkar: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-compact-ipv4-appliance 779715e link false /test e2e-agent-compact-ipv4-appliance
ci/prow/e2e-vsphere-ovn-upi 4fe147f link true /test e2e-vsphere-ovn-upi
ci/prow/okd-e2e-agent-compact-ipv4 c23a3f5 link false /test okd-e2e-agent-compact-ipv4

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants