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

Remove CGO_ENABLED=0 references for FIPS compliance #6504

Closed
wants to merge 7 commits into from
Closed

Remove CGO_ENABLED=0 references for FIPS compliance #6504

wants to merge 7 commits into from

Conversation

rashmigottipati
Copy link
Member

Removes all of the CGO_ENABLED=0 references in order to enable FIPS compliance

@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:26 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:31 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:31 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:33 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:40 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:40 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:42 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:42 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:43 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy July 20, 2023 16:43 — with GitHub Actions Inactive
@joelanford
Copy link
Member

https://github.com/operator-framework/operator-sdk/actions/runs/5613582949/job/15210100650?pr=6504

For the scaffolding, we inherit CGO_ENABLED=0 from Kubebuilder. So we likely need to either:

  • Make this fix in kubebuilder's Go scaffolding plugins and vendor those changes. This could take awhile.
  • Add something in operator-sdk's Go plugin chain that removes CGO_ENABLED=0 from files scaffolded from upstream plugins.

@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 2, 2023 17:59 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 20:27 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 20:27 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 20:27 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 20:27 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 20:27 — with GitHub Actions Inactive
joelanford
joelanford reviewed Aug 3, 2023
View reviewed changes
internal/plugins/manifests/v2/init.go
Comment on lines +78 to +84
//(TODO): This removes `CGO_ENABLED=0` references from the go/v3 and go/v4-alpha scaffolds for FIPS compliance.
// This is a temporary workaround to remove CGO_ENABLED=0 references in the Go plugin chain from files scaffolded from upstream plugins.
// We would need to remove this when the fix is made in upstream Kubebuilder eventually.
err = util.ReplaceInFile("Dockerfile", "CGO_ENABLED=0", "")
if err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

It feels wrong to me to put this functionality in the manifests plugin. Should we have a separate temporary plugin for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you please elucidate what you mean by a separate temporary plugin? Would it be an overkill to have a temporary plugin just for this?

Wondering if having that would eliminate the need to make changes in the Kubebuilder, and if so, that makes sense to me. But if we need to make changes in Kubebuilder anyway, do we still want to have a separate temporary plugin for this?

@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 21:01 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@rashmigottipati rashmigottipati temporarily deployed to deploy August 3, 2023 23:16 — with GitHub Actions Inactive
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 28, 2023
@openshift-merge-robot
Copy link

PR needs rebase.

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/test-infra repository.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 28, 2023
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 27, 2024
@openshift-bot
Copy link

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Feb 27, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 27, 2024

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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/test-infra repository.

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

@joelanford joelanford joelanford left review comments

@oceanc80 oceanc80 Awaiting requested review from oceanc80

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

@everettraven everettraven Awaiting requested review from everettraven

Assignees
No one assigned
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rashmigottipati @joelanford @openshift-merge-robot @openshift-bot