Support uploading to IBM Cloud by FrostyX · Pull Request #1924 · osbuild/images

FrostyX · 2025-10-04T10:28:29Z

See osbuild/image-builder-cli#238

Fix osbuild#238 See osbuild/images#1924

mvo5

Thanks! This looks good, a few nitpicks/suggestions inline.

pkg/cloud/ibmcloud/ibmcloud.go

mvo5 · 2025-10-06T13:11:25Z

pkg/cloud/ibmcloud/ibmcloud.go

+}
+
+func (iu *ibmcloudUploader) Check(status io.Writer) error {
+	return nil


ideally we would do parts of the checks from UploadAndRegister() here too, like checking for the environment. Maybe via a common helper that is then used here and in UploadAndRegister() ?

As per discussion in #1921 (comment) , I am skipping the check for now

Looks like the check only gets called when uploading as part of a build: https://github.com/osbuild/image-builder-cli/blob/392736dac329a3f6ccc6dccd4170b4f20a8f4c1b/cmd/image-builder/main.go#L405
But not called with image-builder upload.

pkg/cloud/ibmcloud/ibmcloud.go

Fix osbuild#238 See osbuild/images#1924

schutzbot · 2025-10-14T11:25:18Z

A previous version of this PR changed the images API or behaviour causing integration issues with osbuild-composer.
This is now fixed.

achilleas-k

Thanks. LGTM.
We can handle the Check() function separately.

supakeen · 2025-10-15T10:03:06Z

Thanks. LGTM. We can handle the Check() function separately.

Did you mean to approve?

thozza

Thanks. My comments can be a follow-up.

thozza · 2025-10-16T12:15:49Z

pkg/cloud/ibmcloud/ibmcloud.go

+	authEndpoint      string
+	apiKey            string
+	crn               string
+	filename          string
+	profilename       string
+	trustedProfileID  string
+	crTokenFilePath   string
+	serviceInstanceID string


Nitpick: Why not use a pointer to credentials.Credentials type and initialize it in NewUploader()?

That's what I would do but for some reason, the AWS uploader does it this

images/pkg/cloud/awscloud/uploader.go

Lines 69 to 71 in 9ff35eb

tags: opts.Tags,

targetArch: opts.TargetArch,

bootMode: opts.BootMode,

so I tried to stay consistent with it.

Thanks for providing more context. The values that you've pointed out from AWS uploader are IMO more like the region, bucketName or imageName, but are optional and not required to be able to upload an image to AWS. Therefore, they are made "optional" by being passed via the UploaderOptions struct.

I still think that my suggestion would make sense in this case and does not go against the consistency in this regard.

Okay, in that case I will try to refactor in the way you are suggesting :-)

Updated, PTAL

thozza · 2025-10-16T12:19:41Z

pkg/cloud/ibmcloud/ibmcloud.go

+	fmt.Fprintf(status, "Uploading to IBM Cloud...\n")
+
+	endpoint := fmt.Sprintf("s3.%s.cloud-object-storage.appdomain.cloud", iu.region)
+	credentials, err := iu.getCredentials()


I think that it would be better to call this in NewUploader() to get an error earlier.

IMHO, it would be a good candidate for the Check method once it gets "fixed"

See osbuild/image-builder-cli#238

achilleas-k

LGTM.

I think this is good to merge and we can do a more general fix-up of the way Check() is handled in uploaders as a follow-up.

I think we should generally add a call to Check() in all UploadAndRegister() implementations, but also keep it separate so that callers can validate the uploader configuration before starting a long-running process (like a build).

FrostyX · 2025-10-20T20:13:41Z

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

The CI failed with

fatal: unable to access 'https://gitlab.com/redhat/services/products/image-builder/ci/images.git/': Failed to connect to gitlab.com

so I guess restarting the tests should help.

achilleas-k · 2025-10-20T20:27:37Z

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

The CI failed with
fatal: unable to access 'https://gitlab.com/redhat/services/products/image-builder/ci/images.git/': Failed to connect to gitlab.com
so I guess restarting the tests should help.

I'll click it again, but with the ongoing AWS outage, I think we might have trouble getting this to go through.

achilleas-k · 2025-10-20T20:28:04Z

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

The CI failed with
fatal: unable to access 'https://gitlab.com/redhat/services/products/image-builder/ci/images.git/': Failed to connect to gitlab.com
so I guess restarting the tests should help.
I'll click it again, but with the ongoing AWS outage, I think we might have trouble getting this to go through.

Then again, this is already up to date with main, so we don't need the merge queue. I'll admin merge.

Changes with 0.205.0 ---------------- - Convert the test config map to a config list (osbuild/images#1900) - Author: Achilleas Koutsou, Reviewers: Sanne Raymaekers, Simon de Vlieger - Repos: Add definitions for AlmaLinux 9.7, 9.8, 10.1 and 10.2 (osbuild/images#1926) - Author: Eduard Abdullin, Reviewers: Achilleas Koutsou, Brian C. Lane, Simon de Vlieger - Support uploading to OpenStack (osbuild/images#1921) - Author: Jakub Kadlčík, Reviewers: Michael Vogt, Simon de Vlieger - disk: make addPartitionsForBootMode() arch specific (osbuild/images#1928) - Author: Michael Vogt, Reviewers: Brian C. Lane, Simon de Vlieger - distro: add ova image type for bootc [HMS-9503] (osbuild/images#1938) - Author: Michael Vogt, Reviewers: Achilleas Koutsou, Simon de Vlieger - fedora/minimal: drop uuids in partition tables (osbuild/images#1932) - Author: Simon de Vlieger, Reviewers: Brian C. Lane, Michael Vogt Changes with 0.206.0 ---------------- - Update osbuild dependency commit ID to latest (osbuild/images#1945) - Author: SchutzBot, Reviewers: Achilleas Koutsou, Simon de Vlieger - deps: bump blueprint to 1.16.0 (osbuild/images#1952) - Author: Simon de Vlieger, Reviewers: Achilleas Koutsou, Sanne Raymaekers - fedora: Drop tigervnc on F42 and later (osbuild/images#1942) - Author: Brian C. Lane, Reviewers: Simon de Vlieger, Tomáš Hozza - fedora: document root kickstart (osbuild/images#1936) - Author: Simon de Vlieger, Reviewers: Brian C. Lane, Michael Vogt - fedora: rawhide is 44 (osbuild/images#1943) - Author: Simon de Vlieger, Reviewers: Achilleas Koutsou, Tomáš Hozza - many: lorax template split (HMS-9524) (osbuild/images#1949) - Author: Simon de Vlieger, Reviewers: Brian C. Lane, Tomáš Hozza Changes with 0.207.0 ---------------- - Enable fedora 43 unit testing (osbuild/images#1954) - Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza - fedora: update cloud_kernel_options (osbuild/images#1953) - Author: Sanne Raymaekers, Reviewers: Achilleas Koutsou, Simon de Vlieger - test/data/repos/rhel-10.2: fix copy & paste error (osbuild/images#1956) - Author: Tomáš Hozza, Reviewers: Achilleas Koutsou, Simon de Vlieger Changes with 0.208.0 ---------------- - Schutzfile: switch CI runner to Fedora 42 (osbuild/images#1955) - Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza - Support uploading to IBM Cloud (osbuild/images#1924) - Author: Jakub Kadlčík, Reviewers: Achilleas Koutsou, Simon de Vlieger - pkg/osbuild: generate osbuild result from status scanner entries (osbuild/images#1941) - Author: Sanne Raymaekers, Reviewers: Nobody

Changes with 0.207.0 ---------------- - Enable fedora 43 unit testing (osbuild/images#1954) - Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza - fedora: update cloud_kernel_options (osbuild/images#1953) - Author: Sanne Raymaekers, Reviewers: Achilleas Koutsou, Simon de Vlieger - test/data/repos/rhel-10.2: fix copy & paste error (osbuild/images#1956) - Author: Tomáš Hozza, Reviewers: Achilleas Koutsou, Simon de Vlieger Changes with 0.208.0 ---------------- - Schutzfile: switch CI runner to Fedora 42 (osbuild/images#1955) - Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza - Support uploading to IBM Cloud (osbuild/images#1924) - Author: Jakub Kadlčík, Reviewers: Achilleas Koutsou, Simon de Vlieger - pkg/osbuild: generate osbuild result from status scanner entries (osbuild/images#1941) - Author: Sanne Raymaekers, Reviewers: Nobody Changes with 0.209.0 ---------------- - gitlab: run ostree manifest generation and builds only when needed (osbuild/images#1961) - Author: Achilleas Koutsou, Reviewers: Simon de Vlieger, Tomáš Hozza - osbuild/osbuild-exec: extract building the osbuild cmd to helper (osbuild/images#1963) - Author: Sanne Raymaekers, Reviewers: Achilleas Koutsou, Simon de Vlieger - rhel10: add ism secret & top secret oscap profiles (HMS-9507) (osbuild/images#1962) - Author: Gianluca Zuccarelli, Reviewers: Lukáš Zapletal, Sanne Raymaekers

Fix osbuild#238 See osbuild/images#1924

Fix #238 See osbuild/images#1924

Fix osbuild#238 See osbuild/images#1924

FrostyX requested a review from a team as a code owner October 4, 2025 10:28

FrostyX requested review from achilleas-k, supakeen and thozza October 4, 2025 10:28

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 4, 2025

Support uploading to IBM Cloud

29bd22d

Fix osbuild#238 See osbuild/images#1924

FrostyX mentioned this pull request Oct 4, 2025

Support uploading to IBM Cloud osbuild/image-builder-cli#338

Merged

FrostyX force-pushed the ibmcloud branch from 6e60884 to 9a763ba Compare October 4, 2025 10:37

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 4, 2025

Support uploading to IBM Cloud

5569637

Fix osbuild#238 See osbuild/images#1924

supakeen requested a review from mvo5 October 6, 2025 12:20

mvo5 reviewed Oct 6, 2025

View reviewed changes

thozza reviewed Oct 13, 2025

View reviewed changes

pkg/cloud/ibmcloud/ibmcloud.go Outdated Show resolved Hide resolved

FrostyX force-pushed the ibmcloud branch from 9a763ba to fa97135 Compare October 14, 2025 11:19

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 14, 2025

Support uploading to IBM Cloud

06ea30f

Fix osbuild#238 See osbuild/images#1924

FrostyX force-pushed the ibmcloud branch from fa97135 to 873deb6 Compare October 14, 2025 11:20

supakeen self-assigned this Oct 14, 2025

achilleas-k reviewed Oct 15, 2025

View reviewed changes

thozza previously approved these changes Oct 16, 2025

View reviewed changes

FrostyX dismissed thozza’s stale review via ef08594 October 17, 2025 12:14

FrostyX force-pushed the ibmcloud branch from 873deb6 to ef08594 Compare October 17, 2025 12:14

Support uploading to IBM Cloud

1614b37

See osbuild/image-builder-cli#238

FrostyX force-pushed the ibmcloud branch from ef08594 to 1614b37 Compare October 17, 2025 12:17

achilleas-k approved these changes Oct 20, 2025

View reviewed changes

supakeen approved these changes Oct 20, 2025

View reviewed changes

supakeen added this pull request to the merge queue Oct 20, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025

achilleas-k merged commit 55c1c85 into osbuild:main Oct 20, 2025
22 of 24 checks passed

croissanne mentioned this pull request Oct 21, 2025

go.mod: update osbuild/images to v0.208.0 osbuild/osbuild-composer#4898

Merged

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 24, 2025

Support uploading to IBM Cloud

45fc231

Fix osbuild#238 See osbuild/images#1924

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 24, 2025

Support uploading to IBM Cloud

a219011

Fix osbuild#238 See osbuild/images#1924

FrostyX added a commit to FrostyX/image-builder-cli that referenced this pull request Oct 27, 2025

Support uploading to IBM Cloud

fe55a6c

Fix osbuild#238 See osbuild/images#1924

github-merge-queue bot pushed a commit to osbuild/image-builder-cli that referenced this pull request Oct 28, 2025

Support uploading to IBM Cloud

4a546a6

Fix #238 See osbuild/images#1924

mvo5 pushed a commit to mvo5/image-builder-cli that referenced this pull request Dec 8, 2025

Support uploading to IBM Cloud

f5264b6

Fix osbuild#238 See osbuild/images#1924

	tags: opts.Tags,
	targetArch: opts.TargetArch,
	bootMode: opts.BootMode,

Conversation

FrostyX commented Oct 4, 2025

Uh oh!

mvo5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

schutzbot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

achilleas-k left a comment

Choose a reason for hiding this comment

Uh oh!

supakeen commented Oct 15, 2025

Uh oh!

thozza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FrostyX Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

achilleas-k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FrostyX commented Oct 20, 2025

Uh oh!

achilleas-k commented Oct 20, 2025

Uh oh!

achilleas-k commented Oct 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

schutzbot commented Oct 14, 2025 •

edited

Loading

FrostyX Oct 16, 2025 •

edited

Loading