Skip to content
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

Remove unnecessary custom conversion functions #77346

Merged
merged 2 commits into from
May 2, 2019

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented May 2, 2019

/kind cleanup

What this PR does / why we need it:

Several custom conversion functions around pod and container security context structs were not required.

Dropping this streamlines addition of new fields to these structs, since the auto-generated conversion will automatically add copies for the new fields, instead of having to manually update the custom functions

Resolves unit test failures in #77147

Does this PR introduce a user-facing change?:

NONE

/sig api-machinery
/cc @sttts

@k8s-ci-robot k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label May 2, 2019
@k8s-ci-robot k8s-ci-robot requested a review from sttts May 2, 2019 16:22
@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. sig/apps Categorizes an issue or PR as relevant to SIG Apps. labels May 2, 2019
@@ -357,88 +357,6 @@ func Convert_v1_Secret_To_core_Secret(in *v1.Secret, out *core.Secret, s convers
return nil
}

func Convert_core_SecurityContext_To_v1_SecurityContext(in *core.SecurityContext, out *v1.SecurityContext, s conversion.Scope) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

compare with autogenerated function:

func autoConvert_core_SecurityContext_To_v1_SecurityContext(in *core.SecurityContext, out *v1.SecurityContext, s conversion.Scope) error {
out.Capabilities = (*v1.Capabilities)(unsafe.Pointer(in.Capabilities))
out.Privileged = (*bool)(unsafe.Pointer(in.Privileged))
out.SELinuxOptions = (*v1.SELinuxOptions)(unsafe.Pointer(in.SELinuxOptions))
out.RunAsUser = (*int64)(unsafe.Pointer(in.RunAsUser))
out.RunAsGroup = (*int64)(unsafe.Pointer(in.RunAsGroup))
out.RunAsNonRoot = (*bool)(unsafe.Pointer(in.RunAsNonRoot))
out.ReadOnlyRootFilesystem = (*bool)(unsafe.Pointer(in.ReadOnlyRootFilesystem))
out.AllowPrivilegeEscalation = (*bool)(unsafe.Pointer(in.AllowPrivilegeEscalation))
out.ProcMount = (*v1.ProcMountType)(unsafe.Pointer(in.ProcMount))
return nil
}

(the Convert_... functions being called were the autogenerated ones)

return nil
}

func Convert_core_PodSecurityContext_To_v1_PodSecurityContext(in *core.PodSecurityContext, out *v1.PodSecurityContext, s conversion.Scope) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

compare with autogenerated function:

func autoConvert_core_PodSecurityContext_To_v1_PodSecurityContext(in *core.PodSecurityContext, out *v1.PodSecurityContext, s conversion.Scope) error {
// INFO: in.HostNetwork opted out of conversion generation
// INFO: in.HostPID opted out of conversion generation
// INFO: in.HostIPC opted out of conversion generation
// INFO: in.ShareProcessNamespace opted out of conversion generation
out.SELinuxOptions = (*v1.SELinuxOptions)(unsafe.Pointer(in.SELinuxOptions))
out.RunAsUser = (*int64)(unsafe.Pointer(in.RunAsUser))
out.RunAsGroup = (*int64)(unsafe.Pointer(in.RunAsGroup))
out.RunAsNonRoot = (*bool)(unsafe.Pointer(in.RunAsNonRoot))
out.SupplementalGroups = *(*[]int64)(unsafe.Pointer(&in.SupplementalGroups))
out.FSGroup = (*int64)(unsafe.Pointer(in.FSGroup))
out.Sysctls = *(*[]v1.Sysctl)(unsafe.Pointer(&in.Sysctls))
return nil
}

(the Convert_... functions being called were the autogenerated ones)

return nil
}

func Convert_v1_PodSecurityContext_To_core_PodSecurityContext(in *v1.PodSecurityContext, out *core.PodSecurityContext, s conversion.Scope) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

compare with autogenerated function:

func autoConvert_v1_PodSecurityContext_To_core_PodSecurityContext(in *v1.PodSecurityContext, out *core.PodSecurityContext, s conversion.Scope) error {
out.SELinuxOptions = (*core.SELinuxOptions)(unsafe.Pointer(in.SELinuxOptions))
out.RunAsUser = (*int64)(unsafe.Pointer(in.RunAsUser))
out.RunAsGroup = (*int64)(unsafe.Pointer(in.RunAsGroup))
out.RunAsNonRoot = (*bool)(unsafe.Pointer(in.RunAsNonRoot))
out.SupplementalGroups = *(*[]int64)(unsafe.Pointer(&in.SupplementalGroups))
out.FSGroup = (*int64)(unsafe.Pointer(in.FSGroup))
out.Sysctls = *(*[]core.Sysctl)(unsafe.Pointer(&in.Sysctls))
return nil
}

(the Convert_... functions being called were the autogenerated ones)

} else {
out.SecurityContext = nil
}
out.SecurityContext = (*core.SecurityContext)(unsafe.Pointer(in.SecurityContext))
Copy link
Member Author

Choose a reason for hiding this comment

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

not defining a custom function means this can collapse to a direct memory copy

} else {
out.SecurityContext = nil
}
out.SecurityContext = (*v1.SecurityContext)(unsafe.Pointer(in.SecurityContext))
Copy link
Member Author

Choose a reason for hiding this comment

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

ditto on direct memory copy

@liggitt
Copy link
Member Author

liggitt commented May 2, 2019

/priority backlog

@k8s-ci-robot k8s-ci-robot added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 2, 2019
@liggitt
Copy link
Member Author

liggitt commented May 2, 2019

/retest

@smarterclayton
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit a6e5cb2 into kubernetes:master May 2, 2019
@liggitt liggitt deleted the remove-custom-conversion branch May 6, 2019 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants