Skip to content

move JSON structs (serialization) to ocm-api-model #1037

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
Merged
merged 4 commits into from
Jun 9, 2025
Merged

move JSON structs (serialization) to ocm-api-model #1037

merged 4 commits into from
Jun 9, 2025

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Mar 25, 2025 •
edited
Loading

remaining

  • Need to add --verify capability

@deads2k deads2k force-pushed the generator-02-alias-api branch 2 times, most recently from 8452228 to 2d7e1e3 Compare March 26, 2025 15:22
@deads2k deads2k changed the title [wip] Generator 02 alias api move JSON structs (serialization) to ocm-api-model Mar 26, 2025
@deads2k deads2k changed the title move JSON structs (serialization) to ocm-api-model [wip] move JSON structs (serialization) to ocm-api-model Mar 26, 2025
@deads2k deads2k force-pushed the generator-02-alias-api branch from 2d7e1e3 to 42b7dd9 Compare March 27, 2025 18:42
@deads2k deads2k mentioned this pull request May 16, 2025
Closed
miguelsorianod
miguelsorianod reviewed May 16, 2025
View reviewed changes
arohcp/v1alpha1/ami_override_builder_alias.go
package v1alpha1 // github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1

import (
api_v1alpha1 "github.com/openshift-online/ocm-api-model/clientapi/arohcp/v1alpha1"
Copy link
Contributor

@miguelsorianod miguelsorianod May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Related question: would the server side (CS) leverage the types defined in github.com/openshift-online/ocm-api-model/clientapi/arohcp/v1alpha1 to perform the deserialization/serialization of the requests/responses?
One of the reasons I ask this is because I observe that the url contains clientapi. Is that intended to be consumed by tooling that builds clients (like th ocm sdk go) only?

Copy link
Contributor

@miguelsorianod miguelsorianod May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Another question:

If one in its codebase decides to use the OCM SDK Go, would they reference the API types defined in the OCM SDK Go (which are type aliases), or the clientapi ones? I understand both ways would work at a technical level as they are type aliases, but one might be more desirable than the other. I am inclined to think about using the ones of the OCM SDK Go as that's what the application would be consuming, but maybe I am missing some advantages on the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related question: would the server side (CS) leverage the types defined in github.com/openshift-online/ocm-api-model/clientapi/arohcp/v1alpha1 to perform the deserialization/serialization of the requests/responses?
One of the reasons I ask this is because I observe that the url contains clientapi. Is that intended to be consumed by tooling that builds clients (like th ocm sdk go) only?

I think it's logical to move the cluster-service server-side serialization to a separate submodule in ocm-api-model so we have one place to watch for API changes and can do things like ensure cross-compatibility between the client and server prior to merging either one, enforced with pre-merge testing.

I do not intend on trying to convert cluster-service to use the clientapi types for serving.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If one in its codebase decides to use the OCM SDK Go, would they reference the API types defined in the OCM SDK Go (which are type aliases), or the clientapi ones?

My first reaction is to use the clientapi package, but I don't think we need to try to change the world immediately. It wouldn't offend my sensibilities to use the type alias in ocm-sdk-go.

@deads2k deads2k force-pushed the generator-02-alias-api branch from 42b7dd9 to 363ace5 Compare May 27, 2025 20:35
@deads2k deads2k changed the title [wip] move JSON structs (serialization) to ocm-api-model move JSON structs (serialization) to ocm-api-model May 27, 2025
@deads2k deads2k force-pushed the generator-02-alias-api branch 2 times, most recently from 798f248 to 9746d6f Compare May 28, 2025 18:57
@miguelsorianod
Copy link
Contributor

miguelsorianod commented May 30, 2025 •
edited
Loading

I did the following test in a local branch I setup using the https://github.com/Azure/ARO-HCP/ repository, where there are a couple of binaries (called backend and frontend) that make a bit of use of the ocm sdk go library:

  • I cloned the branch of this MR locally in my machine. 9746d6f3ee0ba6b434204a9d8f42e89409aded54 commit at the moment of writing this
  • I updated the go.mod of the backend and frontend directories to specify the following replaces:
replace github.com/openshift-online/ocm-sdk-go => /home/msoriano/go/src/github.com/openshift-online/ocm-sdk-go

replace github.com/openshift-online/ocm-api-model/clientapi => github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c

The first one is the replace that points to my local checkout of this MR.

The second one is the same replace as this MR has for the github.com/openshift-online/ocm-api-model/clientapi dependency, as it is not transitive.

I was able to compile the backend and frontend binaries using their respective Make targets. I also applied go mod tidy before.

As a note, for the case of the backend I was able to compile just specifying the ocm-sdk-go replace, but when running go mod tidy an error showed that required the usage of the clientapi module too.

For completeness purposes, this is the git diff of my changes:

diff --git a/backend/go.mod b/backend/go.mod
index 4fba32cc..32a5f9c5 100644
--- a/backend/go.mod
+++ b/backend/go.mod
@@ -64,6 +64,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/openshift-online/ocm-api-model/clientapi v0.0.0-00010101000000-000000000000 // indirect
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
@@ -118,3 +119,7 @@ require (
 )
 
 replace github.com/Azure/ARO-HCP/internal => ../internal
+
+replace github.com/openshift-online/ocm-sdk-go => /home/msoriano/go/src/github.com/openshift-online/ocm-sdk-go
+
+replace github.com/openshift-online/ocm-api-model/clientapi => github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c
diff --git a/backend/go.sum b/backend/go.sum
index 6764f348..008e17a4 100644
--- a/backend/go.sum
+++ b/backend/go.sum
@@ -29,6 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c h1:VAzzDv1EHrVoatgE2qjNVbFs1tbAdk0mm3NJkklRyRk=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c/go.mod h1:fZwy5HY2URG9nrExvQeXrDU/08TGqZ16f8oymVEN5lo=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
 github.com/emicklei/go-restful/v3 v3.12.2 h1:DhwDP0vY3k8ZzE0RunuJy8GhNpPL6zqLkDf9B/a0/xU=
@@ -145,8 +147,6 @@ github.com/onsi/ginkgo/v2 v2.22.2 h1:/3X8Panh8/WwhU/3Ssa6rCKqPLuAkVY2I0RoyDLySlU
 github.com/onsi/ginkgo/v2 v2.22.2/go.mod h1:oeMosUL+8LtarXBHu/c0bx2D/K9zyQ6uX3cTyztHwsk=
 github.com/onsi/gomega v1.36.2 h1:koNYke6TVk6ZmnyHrCXba/T/MoLBXFjeC1PtvYgw0A8=
 github.com/onsi/gomega v1.36.2/go.mod h1:DdwyADRjrc825LhMEkD76cHR5+pUnjhUN8GlHlRPHzY=
-github.com/openshift-online/ocm-sdk-go v0.1.465 h1:RZr92sdcAKyLVcL19/RYOn6KVtspDUH1wc3UuO4LgiE=
-github.com/openshift-online/ocm-sdk-go v0.1.465/go.mod h1:EOkylgH0bafd+SlU9YvMrIIxHJw0Hk1EnC7W1VZeW8I=
 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ=
 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
diff --git a/frontend/go.mod b/frontend/go.mod
index f3cad5f3..8624d75a 100644
--- a/frontend/go.mod
+++ b/frontend/go.mod
@@ -30,6 +30,7 @@ require (
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.1 // indirect
 	github.com/hashicorp/go-version v1.7.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
+	github.com/openshift-online/ocm-api-model/clientapi v0.0.0-00010101000000-000000000000 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/spf13/pflag v1.0.6 // indirect
 	go.opentelemetry.io/auto/sdk v1.1.0 // indirect
@@ -101,3 +102,7 @@ require (
 )
 
 replace github.com/Azure/ARO-HCP/internal => ../internal
+
+replace github.com/openshift-online/ocm-sdk-go => /home/msoriano/go/src/github.com/openshift-online/ocm-sdk-go
+
+replace github.com/openshift-online/ocm-api-model/clientapi => github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c
diff --git a/frontend/go.sum b/frontend/go.sum
index 0f089f8c..68582c68 100644
--- a/frontend/go.sum
+++ b/frontend/go.sum
@@ -33,6 +33,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c h1:VAzzDv1EHrVoatgE2qjNVbFs1tbAdk0mm3NJkklRyRk=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c/go.mod h1:fZwy5HY2URG9nrExvQeXrDU/08TGqZ16f8oymVEN5lo=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78=
 github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc=
 github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU=
@@ -126,8 +128,6 @@ github.com/onsi/ginkgo/v2 v2.22.2 h1:/3X8Panh8/WwhU/3Ssa6rCKqPLuAkVY2I0RoyDLySlU
 github.com/onsi/ginkgo/v2 v2.22.2/go.mod h1:oeMosUL+8LtarXBHu/c0bx2D/K9zyQ6uX3cTyztHwsk=
 github.com/onsi/gomega v1.36.2 h1:koNYke6TVk6ZmnyHrCXba/T/MoLBXFjeC1PtvYgw0A8=
 github.com/onsi/gomega v1.36.2/go.mod h1:DdwyADRjrc825LhMEkD76cHR5+pUnjhUN8GlHlRPHzY=
-github.com/openshift-online/ocm-sdk-go v0.1.465 h1:RZr92sdcAKyLVcL19/RYOn6KVtspDUH1wc3UuO4LgiE=
-github.com/openshift-online/ocm-sdk-go v0.1.465/go.mod h1:EOkylgH0bafd+SlU9YvMrIIxHJw0Hk1EnC7W1VZeW8I=
 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ=
 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
diff --git a/go.work.sum b/go.work.sum
index 3b049999..51a86f8a 100644
--- a/go.work.sum
+++ b/go.work.sum
@@ -1800,6 +1800,8 @@ github.com/danieljoos/wincred v1.2.1/go.mod h1:uGaFL9fDn3OLTvzCGulzE+SzjEe5NGlh5
 github.com/dave/dst v0.27.3 h1:P1HPoMza3cMEquVf9kKy8yXsFirry4zEnWOdYPOoIzY=
 github.com/dave/dst v0.27.3/go.mod h1:jHh6EOibnHgcUW3WjKHisiooEkYwqpHLBSX1iOBhEyc=
 github.com/dave/jennifer v1.7.1/go.mod h1:nXbxhEmQfOZhWml3D1cDK5M1FLnMSozpbFN/m3RmGZc=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c h1:VAzzDv1EHrVoatgE2qjNVbFs1tbAdk0mm3NJkklRyRk=
+github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c/go.mod h1:fZwy5HY2URG9nrExvQeXrDU/08TGqZ16f8oymVEN5lo=
 github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0=
 github.com/denis-tingaikin/go-header v0.5.0 h1:SRdnP5ZKvcO9KKRP1KJrhFR3RrlGuD+42t4429eC9k8=
 github.com/denis-tingaikin/go-header v0.5.0/go.mod h1:mMenU5bWrok6Wl2UsZjy+1okegmwQ3UgWl4V1D8gjlY=
@@ -4523,8 +4525,6 @@ gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
-gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
-gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

@vkareh vkareh requested a review from nimrodshn June 2, 2025 20:37
ahitacat
ahitacat reviewed Jun 5, 2025
View reviewed changes
go.mod Outdated
@@ -63,3 +67,7 @@ require (
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
)

replace github.com/openshift-online/ocm-api-model/clientapi => github.com/deads2k/ocm-api-model/clientapi v0.0.0-20250527201755-64b6c160d65c
Copy link
Contributor

Choose a reason for hiding this comment

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

Leaving a comment here to not forget to remove this replace once the model is merged

ahitacat
ahitacat reviewed Jun 5, 2025
View reviewed changes
ahitacat
ahitacat previously requested changes Jun 5, 2025
View reviewed changes
@deads2k deads2k force-pushed the generator-02-alias-api branch 3 times, most recently from 9335c7f to 6b7a0ec Compare June 6, 2025 19:34
@nimrodshn
Copy link
Contributor

/lgtm
/hold for Victor's review
/assign @vkareh

@deads2k deads2k force-pushed the generator-02-alias-api branch from 6b7a0ec to 2d4cbca Compare June 9, 2025 15:07
@vkareh vkareh dismissed ahitacat’s stale review June 9, 2025 17:28

This is fixed now

@vkareh
Copy link
Member

vkareh commented Jun 9, 2025

Looks good to me!

/approve
/lgtm

@deads2k deads2k removed the request for review from nimrodshn June 9, 2025 17:31
@vkareh vkareh merged commit 27a0b66 into openshift-online:main Jun 9, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miguelsorianod miguelsorianod miguelsorianod left review comments

@vkareh vkareh vkareh approved these changes

@ahitacat ahitacat ahitacat left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@deads2k @miguelsorianod @nimrodshn @vkareh @ahitacat