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.

Sign up for GitHub

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

Jump to bottom

AGENT-876: Authenticate systemd services and curl requests #8393

Merged
merged 2 commits into from
Jul 5, 2024
Merged

AGENT-876: Authenticate systemd services and curl requests #8393

merged 2 commits into from
Jul 5, 2024

Conversation

pawanpinjarkar
Copy link
Contributor

@pawanpinjarkar pawanpinjarkar commented May 10, 2024 •
edited
Loading

  • Set JWT token in the expected env var AGENT_AUTH_TOKEN as needed by assisted service
  • Set authorization header in the API requests
  • Pass AGENT_AUTH_TOKEN env var to required systemd services

@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 10, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 10, 2024 •
edited by openshift-ci bot
Loading

@pawanpinjarkar: This pull request references AGENT-876 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 10, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 10, 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 pawanpinjarkar marked this pull request as ready for review May 10, 2024 20:18
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2024
@pawanpinjarkar
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 10, 2024
@openshift-ci openshift-ci bot requested review from bfournie and zaneb May 10, 2024 20:20
@pawanpinjarkar
Copy link
Contributor Author

/retest-required

@pawanpinjarkar pawanpinjarkar force-pushed the authenticate-systemd branch 3 times, most recently from 34cc96b to 9be9053 Compare May 13, 2024 13:55
@pawanpinjarkar pawanpinjarkar mentioned this pull request May 13, 2024
Merged
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 13, 2024 •
edited by openshift-ci bot
Loading

@pawanpinjarkar: This pull request references AGENT-876 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 as needed by assisted service
  • Set authorization header in the API requests
  • Pass PULL_SECRET_TOKEN env var to required systemd services

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 removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 13, 2024
@openshift-ci openshift-ci bot requested review from andfasano and rwsu May 13, 2024 14:15
@zaneb
Copy link
Member

zaneb commented May 15, 2024

Note to reviewers: this depends on #8395 (specifically this part) I think.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented May 16, 2024 •
edited by openshift-ci bot
Loading

@pawanpinjarkar: This pull request references AGENT-876 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 as needed by assisted service
  • Set authorization header in the API requests
  • Pass AGENT_AUTH_TOKEN env var to required systemd services

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 •
edited
Loading

Note to reviewers: this depends on #8395 (specifically this part) I think.

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 systemd services and other curl requests to API, however, the complete functionality to work end to end depends on below PRs

And most importantly, updating the auth type env var

This was referenced May 16, 2024
Merged
Merged
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2024
@pawanpinjarkar
Copy link
Contributor Author

/retest-required

@pawanpinjarkar pawanpinjarkar force-pushed the authenticate-systemd branch 3 times, most recently from a9e01f9 to 88a4ad5 Compare July 2, 2024 15:03
@pawanpinjarkar
Copy link
Contributor Author

/test e2e-agent-compact-ipv4

1 similar comment
@pawanpinjarkar
Copy link
Contributor Author

/test e2e-agent-compact-ipv4

@pawanpinjarkar
Copy link
Contributor Author

/retest-required

@pawanpinjarkar pawanpinjarkar force-pushed the authenticate-systemd branch 3 times, most recently from 0b47115 to 3064551 Compare July 4, 2024 00:10
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jul 5, 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 e9e6170 link false /test e2e-agent-compact-ipv4-appliance
ci/prow/e2e-vsphere-ovn-upi e9e6170 link true /test e2e-vsphere-ovn-upi
ci/prow/e2e-agent-compact-ipv4-appliance-diskimage 7545d70 link false /test e2e-agent-compact-ipv4-appliance-diskimage
ci/prow/okd-e2e-agent-compact-ipv4 7545d70 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.

sadasu
sadasu reviewed Jul 5, 2024
View reviewed changes
pkg/asset/agent/image/ignition.go
@@ -75,6 +75,7 @@ type agentTemplateData struct {
ConfigImageFiles string
ImageTypeISO string
PublicKeyPEM string
Token string
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Do you want the name to be more specific like AuthToken? Token seems a bit generic.

@sadasu
Copy link
Contributor

sadasu commented Jul 5, 2024

/lgtm

A minor nit. Won't hold the PR for this.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 341dcbf into openshift:master Jul 5, 2024
23 of 25 checks passed
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-installer-altinfra-container-v4.17.0-202407052012.p0.g341dcbf.assembly.stream.el9 for distgit ose-installer-altinfra.
All builds following this will include this PR.

danielerez added a commit to danielerez/appliance that referenced this pull request Sep 26, 2024
@danielerez
Added common.sh with curl wrapper
47916a2 
See: openshift/installer#8393
@danielerez danielerez mentioned this pull request Sep 26, 2024
Merged
danielerez added a commit to openshift/appliance that referenced this pull request Sep 26, 2024
@danielerez
Added common.sh with curl wrapper
d9ca597 
See: openshift/installer#8393
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sadasu sadasu sadasu left review comments

@rwsu rwsu rwsu left review comments

@bfournie bfournie Awaiting requested review from bfournie

@zaneb zaneb Awaiting requested review from zaneb

@andfasano andfasano Awaiting requested review from andfasano

Assignees

@sadasu sadasu

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@pawanpinjarkar @openshift-ci-robot @zaneb @andfasano @deepsm007 @sadasu @openshift-bot @rwsu @openshift-merge-robot