-
Notifications
You must be signed in to change notification settings - Fork 134
Add the cos2cos sample for CodeEngine #196
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.
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
base: main
Are you sure you want to change the base?
Conversation
f7c5043
to
35704c2
Compare
Signed-off-by: Shailza Thakur <[email protected]> Signed-off-by: Hamza Bharmal <[email protected]> Signed-off-by: Subhasree Das <[email protected]> Signed-off-by: Subhasree-Das <[email protected]>
35704c2
to
1ec8829
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for putting so much work into this great sample. All in all it looks very very good. Apart from the comments that I already published, I would like to ask you whether you could some focus on making it really easy for users to get started with this example. Think about which property values can be inferred (if not set). Make assumptions. Be opinionated. Keep it easy for users to get it going... While more complex configurations should be applicable, we shouldn't confront all users with it right-away.
Keep up the great work and thanks for this contribution!!!
code-engine-cos2cos/Dockerfile
Outdated
@@ -0,0 +1,25 @@ | |||
FROM golang:1.23-alpine as builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, can you please adjust the Dockerfile and make sure that a) it does not pull any artifact from dockerhub (if no hostname is specified docker.io is used) and by the multi-staged build uses a distroless image in the end?
You can take the following one as an example
https://github.com/IBM/CodeEngine/blob/main/trusted-profiles/go/Dockerfile
code-engine-cos2cos/README.md
Outdated
### Setup & Configuration | ||
1. **Clone the Repository**: | ||
```bash | ||
git clone https://github.ibm.com/ibmcloud-codeengine-internship/code-engine-cos2cos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This URL must be adjusted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed as follows
bash
git clone https://github.com/IBM/CodeEngine.git
cd code-engine-cos2cos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, that I read this, I noticed that we should call the folder cos2cos
. Please wait with that renaming until we processed the entire review, otherwise this will be a mess.
However, in this PR the readme should already be adjusted
git clone https://github.com/IBM/CodeEngine.git
cd cos2cos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We agree with your point. For now we have updated the readme with above changes.
code-engine-cos2cos/README.md
Outdated
#### 2. Build Using Source Code (Local Source) | ||
|
||
To build the image from local source code using IBM Cloud Code Engine: | ||
|
||
```bash | ||
ibmcloud ce build create --name ${BUILD_NAME} --build-type local --image ${REGISTRY}/${NAMESPACE}/${IMAGE_NAME} | ||
ibmcloud ce buildrun submit --build ${BUILD_NAME} --name ${BUILD_NAME}-build-run | ||
``` | ||
|
||
#### 3. Build Using Git-based Source | ||
|
||
To build the image using a Git repository: | ||
|
||
1. Create a deploy key or user-access key in your GitHub repository. | ||
2. Add the private key by creating an SSH secret in IBM Cloud Code Engine. | ||
3. Create a build using the Git repository: | ||
|
||
```bash | ||
ibmcloud ce build create \ | ||
--name ${BUILD_NAME} \ | ||
--image ${REGISTRY}/${NAMESPACE}/${IMAGE_NAME} \ | ||
--source ${GIT_SSH_URL} \ | ||
--context-dir / \ | ||
--strategy dockerfile \ | ||
--git-repo-secret ${GIT_SSH_SECRET} | ||
``` | ||
|
||
4. Submit the build: | ||
|
||
```bash | ||
ibmcloud ce buildrun submit --build ${BUILD_NAME} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to make use of our integrated build and building the container image along with creating the job template:
ibmcloud ce job create
--name ... \
--src "." \
--wait
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change implemented in config.sh file from line number 185.
// ginkgo.Context("should handle errors", func() { | ||
|
||
// }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the code isn't needed, please remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fast turnarounds. Really appreciate the effort.
I will take a look at mechanics of this asset more closely while trying it out.
code-engine-cos2cos/Dockerfile
Outdated
# Stage 2: Final stage (minimal image) | ||
FROM gcr.io/distroless/static-debian12 | ||
|
||
# RUN apk --no-cache add ca-certificates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not needed, would you mind to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
code-engine-cos2cos/main.go
Outdated
os.Setenv("IBM_COS_CRTokenFilePath_PRIMARY", "/var/run/secrets/codeengine.cloud.ibm.com/compute-resource-token/token") | ||
os.Setenv("IBM_COS_CRTokenFilePath_SECONDARY", "/var/run/secrets/codeengine.cloud.ibm.com/compute-resource-token/token") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Trusted Profile token path will always be the same, I don't see a reason to specify it twice and make it configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we are using a single trusted profile and we have also changed the CRTokenFilePath to a static variable within the code and accordingly made the changes in main.go.
code-engine-cos2cos/config.sh
Outdated
--from-literal IBM_COS_CRTokenFilePath_PRIMARY=${IBM_COS_CRTokenFilePath_PRIMARY} \ | ||
--from-literal IBM_COS_CRTokenFilePath_SECONDARY=${IBM_COS_CRTokenFilePath_SECONDARY} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the need to have the token path configurable, please just use a static variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed as requested to a single variable.
code-engine-cos2cos/config.sh
Outdated
--from-literal IBM_COS_TRUSTED_PROFILE_ID_PRIMARY=${COS_TRUSTED_PROFILE_ID_PRIMARY} \ | ||
--from-literal IBM_COS_TRUSTED_PROFILE_ID_SECONDARY=${COS_TRUSTED_PROFILE_ID_SECONDARY} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For complexity reasons, please use a single trusted profile that grants permissions to the primary and secondary COS bucket. In the blog post that I wrote about trusted profiles, I just split it up into two trusted profiles to make it clear that is possible in general
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed as requested to a single variable.
Note:- There has been a slight modification in main.go specifically while creating the COS client. Instead of fetching CRTokenFIlePath for primary and secondary from env file, a static value is passed which is mentioned in main.go
code-engine-cos2cos/config.sh
Outdated
--env-from-secret ${AUTH_SECRET} \ | ||
--argument true 2>/dev/null \ | ||
--wait | ||
# --registry-secret ${CONTAINER_REGISTRY_SECRET} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall be removed as not longer needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
code-engine-cos2cos/config.sh
Outdated
# echo "Job '${JOB_NAME}' already exists. Exiting" | ||
# exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall be removed as not longer needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this line, we have added an if statement asking the user if they want to override the existing job or create a new one.
code-engine-cos2cos/config.sh
Outdated
--env-from-secret ${BASE_SECRET} \ | ||
--env-from-secret ${AUTH_SECRET} \ | ||
--argument true 2>/dev/null | ||
# --registry-secret ${CONTAINER_REGISTRY_SECRET} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall be removed as not longer needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
code-engine-cos2cos/env_sample
Outdated
|
||
# Primary Bucket Trusted Profile Credentials | ||
IBM_COS_TRUSTED_PROFILE_ID_PRIMARY= <> | ||
IBM_COS_CRTokenFilePath_PRIMARY=/var/run/secrets/tokens/service-account-token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to remove that one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have removed the env_sample file since it is no longer in use.
code-engine-cos2cos/config.sh
Outdated
echo "Step 8.4: Compute Resource Token" | ||
curl \ | ||
--request PATCH "https://api.${PROJECT_REGION}.codeengine.cloud.ibm.com/v2/projects/$(ibmcloud ce project current --output json | jq -r .guid)/jobs/${JOB_NAME}" \ | ||
--header 'Accept: application/json' \ | ||
--header "Authorization: $(ibmcloud iam oauth-tokens --output json | jq -r '.iam_token')" \ | ||
--header 'Content-Type: application/merge-patch+json' \ | ||
--header 'If-Match: *' \ | ||
--data-raw "{ | ||
\"run_compute_resource_token_enabled\": true | ||
}" 2>/dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am not mistaken, this code can be replaced with the corresponding option in the job create/update operation, as we added support for trusted profiles for jobs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing this out. We replaced it with option in job create/update --trusted-profile-enabled.
This application processes objects (files) in IBM Cloud Object Storage (COS) between a primary and secondary bucket. The sample can be tried out using CodeEngine.
Copied from https://github.ibm.com/ibmcloud-codeengine-internship/code-engine-cos2cos.