DeepCopy is not needed in the conversion code

[capri-xiyue]

Problem
For the conversion code, the code style is inconsistent, sometimes we use DeepCopy, sometimes we don't.
Examples:

eventing/pkg/apis/sources/v1alpha1/apiserver_conversion.go

Line 119 in a0a5daf

sink.Spec.CloudEventOverrides = source.Spec.CloudEventOverrides

eventing/pkg/apis/sources/v1alpha1/apiserver_conversion.go

Line 88 in a0a5daf

if source.Spec.CloudEventOverrides != nil {

eventing/pkg/apis/sources/v1alpha1/apiserver_conversion.go

Line 68 in a0a5daf

if source.Spec.ResourceOwner != nil {

As we discussed in this PR, #3616 (comment)
, DeepCopy is not needed in the conversion code. We can get rid of all DeepCopy in conversion code.

Exit Criteria
get rid of all DeepCopy in all conversion code.

Time Estimate (optional):
How many developer-days do you think this may take to resolve?

Additional context (optional)
Add any other context about the feature request here.

[capri-xiyue]

This is a good-first-issue

[capri-xiyue]

/good-first-issue

[knative-prow-robot]

@capri-xiyue:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

Details

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[n3wscott]

This is a pretty tricky issue, you have to understand which things are copying memory and which things are just copying pointers. If you just copy the pointers, then later in the code you modify the original version, you will mistakenly also update the converted version, or vice versa.

If you do remove the deepcopy, tests need to be added to make sure the original object and the new object are not linked in memory and there is a unintended mutation.

[vaikas]

Yeah, as @n3wscott says we have to be little careful about it. For example in places where we modify things (like, set the annotations / labels for example to indicate the channelable version, we do have to make deep copy, of at least some fields.

[lionelvillard]

To be clear, the ConvertTo and ConvertFrom contract does not require DeepCopy(). That's said, sure you need to be careful about it!

[capri-xiyue]

I just checked some docs.
It looks like "Conversion functions must not mutate the source object". As I've mentioned in this comments:
#3616 (comment).
"as long as the object is a pointer or the struct contains pointer or array, we should use deep copy when doing conversion."
If that's this case, I think we can just close the issue.