Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

78 changes: 78 additions & 0 deletions mantle/cmd/ore/ibmcloud/copy-object.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright 2021 RedHat.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package ibmcloud

import (
"fmt"
"os"

"github.com/spf13/cobra"
)

var (
cmdCopyObject = &cobra.Command{
Use: "copy-object",
Short: "Copy IBMCloud object to a bucket",
Long: "Copy an IBMCloud object from a bucket in a region to another bucket in a specified region",
Example: ` ore ibmcloud copy-object --source-name=image.qcow2 \
--destination-region=us-south \
--destination-bucket=coreos-dev-image-ibmcloud-us-south`,
RunE: runCopyObject,

SilenceUsage: true,
}

copyCloudObjectStorage string
sourceBucket string
sourceName string
destRegion string
destBucket string
)

func init() {
IbmCloud.AddCommand(cmdCopyObject)
cmdCopyObject.Flags().StringVar(&copyCloudObjectStorage, "cloud-object-storage", "coreos-dev-image-ibmcloud", "cloud object storage to be used")
cmdCopyObject.Flags().StringVar(&sourceBucket, "source-bucket", "coreos-dev-image-ibmcloud-us-east", "bucket where object needs to be copied from")
cmdCopyObject.Flags().StringVar(&sourceName, "source-name", "", "name of object to be copied")
cmdCopyObject.MarkFlagRequired("source-name")
cmdCopyObject.Flags().StringVar(&destRegion, "destination-region", "", "region to be copied to")
cmdCopyObject.MarkFlagRequired("destination-region")
cmdCopyObject.Flags().StringVar(&destBucket, "destination-bucket", "", "destination bucket to copy to")
cmdCopyObject.MarkFlagRequired("destination-bucket")
}

func runCopyObject(cmd *cobra.Command, args []string) error {
if err := API.NewS3Client(copyCloudObjectStorage, destRegion); err != nil {
return err
}

bucketExists, err := API.CheckBucketExists(destBucket)
if err != nil {
return err
}

if !bucketExists {
if err = API.CreateBucket(destBucket); err != nil {
return err
}
}
Comment thread
Prashanth684 marked this conversation as resolved.

if err = API.CopyObject(sourceBucket, sourceName, destBucket); err != nil {
fmt.Fprintf(os.Stderr, "Couldn't copy objects: %v\n", err)
os.Exit(1)
}

return nil
}
27 changes: 27 additions & 0 deletions mantle/platform/api/ibmcloud/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ package ibmcloud
import (
"fmt"
"io"
"net/url"
"time"

"github.com/IBM-Cloud/bluemix-go/api/resource/resourcev2/controllerv2"
"github.com/IBM/ibm-cos-sdk-go/aws"
"github.com/IBM/ibm-cos-sdk-go/aws/awserr"
"github.com/IBM/ibm-cos-sdk-go/aws/credentials/ibmiam"
"github.com/IBM/ibm-cos-sdk-go/aws/session"
"github.com/IBM/ibm-cos-sdk-go/service/s3"
Expand Down Expand Up @@ -164,3 +166,28 @@ func (a *API) UploadObject(r io.Reader, objectName, bucketName string, force boo
plog.Infof("Upload completed successfully in %f seconds to location %s\n", time.Since(startTime).Seconds(), result.Location)
return err
}

// CopyObject - Copy an Object to a new location
func (a *API) CopyObject(srcBucket, srcName, destBucket string) error {
_, err := a.s3client.s3Session.CopyObject(&s3.CopyObjectInput{
CopySource: aws.String(url.QueryEscape(fmt.Sprintf("%s/%s", srcBucket, srcName))),
Bucket: aws.String(destBucket),
Key: aws.String(srcName),
})
if err != nil {
if awserr, ok := err.(awserr.Error); ok {
if awserr.Code() == "BucketAlreadyOwnedByYou" {
return nil
}
}
}

// Wait to see if the item got copied
err = a.s3client.s3Session.WaitUntilObjectExists(&s3.HeadObjectInput{Bucket: aws.String(destBucket), Key: aws.String(srcName)})
if err != nil {
return fmt.Errorf("Error occurred while waiting for item %q to be copied to bucket %q, %v", srcName, destBucket, err)
}

plog.Infof("Item %q successfully copied from bucket %q to bucket %q\n", srcName, srcBucket, destBucket)
return err
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions schema/cosa/cosa_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Build struct {
FedoraCoreOsParentVersion string `json:"fedora-coreos.parent-version,omitempty"`
Gcp *Gcp `json:"gcp,omitempty"`
GitDirty string `json:"coreos-assembler.config-dirty,omitempty"`
IbmCloud *Cloudartifact `json:"ibmcloud,omitempty"`
IbmCloud []Cloudartifact `json:"ibmcloud,omitempty"`
ImageInputChecksum string `json:"coreos-assembler.image-input-checksum,omitempty"`
InputHasOfTheRpmOstree string `json:"rpm-ostree-inputhash"`
Koji *Koji `json:"koji,omitempty"`
Expand All @@ -70,7 +70,7 @@ type Build struct {
OverridesActive bool `json:"coreos-assembler.overrides-active,omitempty"`
PkgdiffAgainstParent PackageSetDifferences `json:"parent-pkgdiff,omitempty"`
PkgdiffBetweenBuilds PackageSetDifferences `json:"pkgdiff,omitempty"`
PowerVirtualServer *Cloudartifact `json:"powervs,omitempty"`
PowerVirtualServer []Cloudartifact `json:"powervs,omitempty"`
ReleasePayload *Image `json:"release-payload,omitempty"`
}

Expand Down Expand Up @@ -103,7 +103,8 @@ type BuildArtifacts struct {

type Cloudartifact struct {
Bucket string `json:"bucket,omitempty"`
Image string `json:"image"`
Image string `json:"image,omitempty"`
Object string `json:"object,omitempty"`
Comment on lines +106 to +107
Copy link
Copy Markdown
Member

@dustymabe dustymabe Sep 21, 2021

Choose a reason for hiding this comment

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

@jlebon @cgwalters - I suggested @Prashanth684 add an object field here so they can identify things via bucket/object rather than re-use the image key here (which typically implies bootable image (like AMI)). Sound OK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, feels like we should just get rid of the generic Cloudartifact, and have separate AzureArtifact, IbmCloudArtifact, and PowerVirtualServerArtifacts types. WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah, he previously had something like IbmCloudArtifact to represent both IBMCloud and PowerVS but I suggested he use CloudArtifact (at the time we hadn't proposed adding the new object field). I honestly think we don't really need that. Feels like re-using cloudArtifact here should be fine since it shares concepts with other clouds (unlike GCP, which has projects and image families and such).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jlebon thoughts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So let me make sure I understand:

  • Azure uses only Image and URL
  • IbmCloud uses Bucket, Object, Region, and Url
  • PowerVirtualServer uses Bucket and Object

?

I mean, it depends how "well-typed" we want to be. Those clearly could use separate types, and then validation would be that much stronger (which means e.g. we don't run the risk of pushing an azure key with fields which don't make sense). While in FCOS these details are mostly internal, because we have layers of metadata on top with very different schemas, in RHCOS the meta.json is only lightly transformed and shoved into the installer binary. Though it probably wouldn't really get far into CI if the required fields were actually wrong.

So overall, I'd lean towards tighter typing, but at the same time I don't think it's a pressing issue (and ideally eventually we stop hardcoding meta.json in the installer). So I wouldn't block on it either.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So let me make sure I understand:

* `Azure` uses only `Image` and `URL`

looks like it, though I will admit we don't use this type for FCOS so I don't know if the image: that's getting populated in the metadata for RHCOS is an actual bootable image or just the file name of the object in object storage.

* `IbmCloud` uses `Bucket`, `Object`, `Region`, and `Url`> 
* `PowerVirtualServer` uses `Bucket` and `Object`

IIUC both of these would populate Bucket, Object, Region, and Url. Obviously knowing enough about the structure of the cloud you can introspect URL from the other data. It's mostly there as a nice to have.

I mean, it depends how "well-typed" we want to be. Those clearly could use separate types, and then validation would be that much stronger (which means e.g. we don't run the risk of pushing an azure key with fields which don't make sense). While in FCOS these details are mostly internal, because we have layers of metadata on top with very different schemas, in RHCOS the meta.json is only lightly transformed and shoved into the installer binary. Though it probably wouldn't really get far into CI if the required fields were actually wrong.

So overall, I'd lean towards tighter typing, but at the same time I don't think it's a pressing issue (and ideally eventually we stop hardcoding meta.json in the installer). So I wouldn't block on it either.

I honestly think we need to take a step back and make sure all of our metadata variants make more sense.

For now I propose we just go forward with what we have here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For now I propose we just go forward with what we have here.

Yup sure, WFM!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes so IBMCloud artifacts and PowerVS artifacts would have the same fields as they both use the same cloud and similar object storage buckets.

Region string `json:"region,omitempty"`
URL string `json:"url"`
}
Expand Down
32 changes: 22 additions & 10 deletions schema/cosa/schema_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ var generatedSchemaJSON = `{
"cloudartifact": {
"type": "object",
"required": [
"image",
"url"
],
"optional": [
"image",
"object",
"bucket",
"region"
],
Expand All @@ -101,6 +102,11 @@ var generatedSchemaJSON = `{
"$id":"#/cloudartifact/region",
"type":"string",
"title":"Region"
},
"object": {
"$id":"#/cloudartifact/object",
"type":"string",
"title":"Object"
}
}
},
Expand Down Expand Up @@ -178,7 +184,7 @@ var generatedSchemaJSON = `{
}
},
"$schema":"http://json-schema.org/draft-07/schema#",
"$id":"https://github.com/coreos/coreos-assembler/blob/main/schema/v1.json",
"$id":"https://github.com/coreos/coreos-assembler/blob/main/v1.json.json",
Comment thread
Prashanth684 marked this conversation as resolved.
"type":"object",
"title":"CoreOS Assember v1 meta.json schema",
"required": [
Expand Down Expand Up @@ -820,16 +826,22 @@ var generatedSchemaJSON = `{
}
},
"ibmcloud": {
"$id":"#/properties/ibmcloud",
"type":"object",
"title":"IBM Cloud",
"$ref": "#/definitions/cloudartifact"
"$id":"#/properties/ibmcloud",
"type":"array",
"title":"IBM Cloud",
"items": {
"type":"object",
"$ref": "#/definitions/cloudartifact"
}
},
"powervs": {
"$id":"#/properties/powervs",
"type":"object",
"title":"Power Virtual Server",
"$ref": "#/definitions/cloudartifact"
"$id":"#/properties/powervs",
"type":"array",
"title":"Power Virtual Server",
"items": {
"type":"object",
"$ref": "#/definitions/cloudartifact"
}
},
"release-payload": {
"$id":"#/properties/release-payload",
Expand Down
Loading