buildpacks / pack

CLI for building apps using Cloud Native Buildpacks
https://buildpacks.io
Apache License 2.0
2.54k stars 286 forks source link

An option to specify the timestamp upon rebasing #1931

Open kessibi opened 11 months ago

kessibi commented 11 months ago

Description

One of the key feature in my use of buildpack is the ability to effortlessly rebase. One of the great pillar stone missing is the ability to provide a timestamp just like the --creation-time on build. If this has been discussed in the past, what is the reason behind not implementing it?

Proposed solution

Add the possibility to give a proper timestamp/string as a date to a rebased image. Example: --rebase-time or just --creation-time to stay consistent with podman/docker output standards.

Describe alternatives you've considered

I have looked into doing it manually fell short. I think changing the creation time would change the image signature sha.

Thank you.

natalieparellano commented 11 months ago

I think we can support this. We already do for pack build, it can work similarly for pack rebase. All we would need to do is set SOURCE_DATE_EPOCH in the lifecycle's execution environment.

--creation-time string         Desired create time in the output image config. Accepted values are Unix timestamps (e.g., '1641013200'), or 'now'.
kessibi commented 11 months ago

Maybe while rebasing it could be good to at least remember the creation time of the pack build phase. Right now, building with --creation-time and then rebasing just nulls the creation time. Thank you for considering this addition either way.

natalieparellano commented 11 months ago

@kessibi that's a great point! Maybe we could introduce a special value preserve (or something) for rebase that preserves the original creation time, whatever it was. We already have special value now for pack build so this would be an extension of that.

PratikforCoding commented 10 months ago

Hey I want to work on this issue! @natalieparellano can you please suggest me the files where I can improve to make it work.

PratikforCoding commented 10 months ago

I think the sourceDateEpochEnv in the lifecycle's execution environment is already set by this:

withEnv := NullOp()
if l.opts.CreationTime != nil && l.platformAPI.AtLeast("0.9") {
    withEnv = WithEnv(fmt.Sprintf("%s=%s", sourceDateEpochEnv, strconv.Itoa(int(l.opts.CreationTime.Unix()))))
}

in this https://github.com/buildpacks/pack/blob/b12c9b3a0a8b3e9e0df590eaf901af44e5c82f06/internal/build/lifecycle_execution.go#L357

can you guide me about rebase? or needed file where change should be made?

jjbustamante commented 10 months ago

I think the sourceDateEpochEnv in the lifecycle's execution environment is already set by this:

Yes, but as you can see, we are setting the environment variable when running:

In this case the idea is to add a new behavior for the rebase command.

  1. The entry point for the command, if you need to add a new flag, is here
  2. Pack handle the concept of a client implementation, which is taking care of the heavy lifting of implement the commands, you will find the rebase implementation here
  3. I am not very familiar with the rebase code base, we need to check here to find out how to implement the new behavior!

I will try to keep checking and guide you through the process.

jjbustamante commented 10 months ago

Checking a little bit the code, here are my thoughts (this is just my initial understanding), @natalieparellano give me a hand if I am wrong!

Again, these are just some ideas, but I am not 100% sure about it.

PratikforCoding commented 10 months ago

@jjbustamante , @natalieparellano can you point me where this particular rebase method is : https://github.com/buildpacks/lifecycle/blob/33c5b890044980b25810c0719801af0398fe9b3b/phase/rebaser.go#L61

        // rebase
    if err = workingImage.Rebase(origMetadata.RunImage.TopLayer, newBaseImage); err != nil {
        return files.RebaseReport{}, fmt.Errorf("rebase app image: %w", err)
    }
jjbustamante commented 10 months ago

Hi @PratikforCoding

  1. We have a library called imgutil which is like an utility library for pack and lifecycle, we define an Image interface with the operations we need for an Image. In this case, the Rebase operation is one of them.
  2. We have different implementations for this interface:
    • local: the implementation to deal with the docker daemon
    • remote: the implementation to deal with OCI registries
    • layout: the implementation to deal with the images saved on disk in OCI layout format
  3. For the Rebase operation, you can find the implementation in the local and remote folder
  4. We don't have any implementation for the Rebase operation in the layout image
natalieparellano commented 10 months ago

Apologies for the slow reply - since pack is using the lifecycle as a library (see here) I think if you just set SOURCE_DATE_EPOCH in pack's execution environment it should just work without any code changes. If I'm right, maybe all we need to do is provide helpers for a better UX when the desired value is now or keep/preserve or whatever.

Edit: this is not true. We need to instantiate the image with WithCreatedAt or call SetCreatedAt as mentioned in the following comments.

PratikforCoding commented 9 months ago

Hey @jjbustamante @natalieparellano, after inspecting the files and imgutil library , I think the feature can be implemented by this:

  1. Implementing a RebaseOption struct here containing CreationTime variable.
  2. Implementing SetCreatedAt setter function in local and remote file that is local.go and remote.go.
  3. Then we have to pass the SetCreatedAt function using Image interface here.
  4. Then we can manipulate the CreationTime of the appImage using the setter and getter method of imgutil library here.

I guess this should work, help me to understand if my approach is wrong.

jjbustamante commented 9 months ago

Hi @PratikforCoding

I will try to go through your comment.

  1. Implementing a RebaseOption struct here containing CreationTime variable.

Yes, actually here the rebase client options structure.

  1. Implementing SetCreatedAt setter function in local and remote file that is local.go and remote.go.

What we do to set the CreatedAt is, we do it during Image creation using the ImageOption.

Basically we need to pass through the option when the remote or local image is created. Now, our problem is to find where this is happening during the rebase operation. Checking Natalie's comment. When we run pack build --creation-time:

I think we need to do a similar logic for rebase. Checking the code in the lifecycle, we are creating the Image instances here, as you can see, it is very similar to what we do in the exporter phase, we need to have available the custom creation time.

An option to do this will be:

  1. As Natalie said, we set the environment variable SOURCE_DATE_EPOCH on pack side with the value provided by the end-user
  2. On lifecycle:
    • We read the environment variable in the lifecycle, with a method similar to the one we have in the exporter (or maybe we refactor that one to be reuse)
    • We use this value to update the image creation logic in the rebase
  1. Then we have to pass the SetCreatedAt function using Image interface here.
  2. Then we can manipulate the CreationTime of the appImage using the setter and getter method of imgutil library here.

I am not sure, but I think we try to avoid those setters/getters on values that we don't expect to change on a Image, that's why I think the way will be to set the value during the image creation time.

jjbustamante commented 7 months ago

Hi @PratikforCoding

Are you still interest in working on this issue? I asked because I am planning next release and I want to see if I can add it into the scope of 0.34.0?

PratikforCoding commented 7 months ago

Hi @jjbustamante sir,

I want to work on this issue but I was struggling and my university exam started. I am still interested to work in this issue, can you tell me in how many days the 0.34.0 will be released?

jjbustamante commented 7 months ago

Hi @PratikforCoding, well, my plan is to have some release candidate version ready for next KubeCon EU, March 18 - 22

PratikforCoding commented 7 months ago

Okay @jjbustamante , can I take time till start of march? I will try to implement this.

kessibi commented 2 months ago

Good day, has their been progress on this issue? Thanks.

natalieparellano commented 2 months ago

Hi @kessibi I don't believe so, but we would welcome a PR for this. The guidance provided here seems fine to me, although I'd recommend instantiating the image with the WithCreatedAt option instead of modifying it after-the-fact.