Skip to content

Create an 'apis' golang module to minimize dependencies for clients#3231

Closed
dprince wants to merge 2 commits into
openshift:masterfrom
dprince:api_module
Closed

Create an 'apis' golang module to minimize dependencies for clients#3231
dprince wants to merge 2 commits into
openshift:masterfrom
dprince:api_module

Conversation

@dprince
Copy link
Copy Markdown

@dprince dprince commented Jul 6, 2022

- What I did

Create new apis module so that external projects can leveral the MCO API structs without importing the larger set of dependencies at the projects top level go.mod file.

- How to verify it

Once landed projects can import github.com/openshift/machine-config-operator/pkg/apis instead of github.com/openshift/machine-config-operator (~90% reduction in go.sum file contents...)

- Description for the changelog

This patch creates a new './pkg/apis' go module with
the existing MCO API which makes it possible for
external projects to import the API structs for
MCO without the larger set of dependencies in the projects
top level go.mod

See notes on multiple go.mod files in the same repository here:
https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository
(See the "Two example scenarios where it can make sense to have more than one go.mod in a repository" section).

dprince added 2 commits July 6, 2022 10:55
This patch creates a new './pkg/apis' go module with
the existing MCO API. This makes it possible for
external projects to import the API structs for
MCO without the larger set of dependencies in the projects
top level go.mod
@openshift-ci openshift-ci Bot requested review from sinnykumari and yuqi-zhang July 6, 2022 15:05
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 6, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dprince
To complete the pull request process, please assign sinnykumari after the PR has been reviewed.
You can assign the PR to them by writing /assign @sinnykumari in a comment when ready.

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

Details 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
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jul 6, 2022

@dprince: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@sinnykumari
Copy link
Copy Markdown
Contributor

Many thanks for the PR!

I am not sure if we want to go in this direction. Ideally MCO APIs and CRDs has to be moved to https://github.com/openshift/api repo where majority of OpenShift related apis lives. There has been multiple attempts from MCO team to perform this move, most recent one was in openshift/api#1060 . This is still in our radar and we would like to make switch once we are able to prioritize the work.
Can you help us understand if it will still make sense or we should close this in favor of moving MCO APIs to centralized openshift/api location?

@dprince
Copy link
Copy Markdown
Author

dprince commented Jul 8, 2022

Can you help us understand if it will still make sense or we should close this in favor of moving MCO APIs to centralized openshift/api location?

I think either pattern is valid. This was quite easy to implement. It solves the problem albeit leaving them in this repo instead of moving to openshift/api. It also seems to adhere to exceptions for when you might want multiple modules in a repo:

https://github.com/golang/go/wiki/Modules#should-i-have-multiple-modules-in-a-single-repository

That said if the decision above is to use openshift/api that works too. Would be nice to have it sooner rather than later though as the security patches due to some of the MCO dependencies are becoming noticable. Is someone actively working on the effort still? If not perhaps I can help.

@dprince dprince closed this Jul 8, 2022
@sinnykumari
Copy link
Copy Markdown
Contributor

Is someone actively working on the effort still? If not perhaps I can help.

Not at the moment. If you can help, that will be great. Let us know if you would like to work on it :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants