Skip to content

Commit

Permalink
Merge pull request kubernetes#103676 from kebe7jun/automated-cherry-p…
Browse files Browse the repository at this point in the history 
…ick-of-#103284-upstream-release-1.21

Automated cherry pick of kubernetes#103284: Fix the code is leaking the defaulting between unrelated pod
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Aug 6, 2021
2 parents 25c25b6 + 8ceaecb commit d7686a8
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 23 deletions.
33 changes: 17 additions & 16 deletions staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,31 +76,34 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
case *corev1.Pod:
// if allContainers is true, then we're going to locate all containers and then iterate through them. At that point, "allContainers" is false
if !allContainers {
currOpts := new(corev1.PodLogOptions)
if opts != nil {
opts.DeepCopyInto(currOpts)
}
// in case the "kubectl.kubernetes.io/default-container" annotation is present, we preset the opts.Containers to default to selected
// container. This gives users ability to preselect the most interesting container in pod.
if annotations := t.GetAnnotations(); annotations != nil && len(opts.Container) == 0 {
var containerName string
if annotations := t.GetAnnotations(); annotations != nil && currOpts.Container == "" {
var defaultContainer string
if len(annotations[podcmd.DefaultContainerAnnotationName]) > 0 {
containerName = annotations[podcmd.DefaultContainerAnnotationName]
defaultContainer = annotations[podcmd.DefaultContainerAnnotationName]
} else if len(annotations[defaultLogsContainerAnnotationName]) > 0 {
// Only log deprecation if we have only the old annotation. This allows users to
// set both to support multiple versions of kubectl; if they are setting both
// they must already know it is deprecated, so we don't need to add noisy
// warnings.
containerName = annotations[defaultLogsContainerAnnotationName]
defaultContainer = annotations[defaultLogsContainerAnnotationName]
fmt.Fprintf(os.Stderr, "Using deprecated annotation `kubectl.kubernetes.io/default-logs-container` in pod/%v. Please use `kubectl.kubernetes.io/default-container` instead\n", t.Name)
}
if len(containerName) > 0 {
if exists, _ := podcmd.FindContainerByName(t, containerName); exists != nil {
opts.Container = containerName
if len(defaultContainer) > 0 {
if exists, _ := podcmd.FindContainerByName(t, defaultContainer); exists == nil {
fmt.Fprintf(os.Stderr, "Default container name %q not found in pod %s\n", defaultContainer, t.Name)
} else {
fmt.Fprintf(os.Stderr, "Default container name %q not found in a pod\n", containerName)
currOpts.Container = defaultContainer
}
}
}

var containerName string
if opts == nil || len(opts.Container) == 0 {
if currOpts.Container == "" {
// We don't know container name. In this case we expect only one container to be present in the pod (ignoring InitContainers).
// If there is more than one container, we should return an error showing all container names.
if len(t.Spec.Containers) != 1 {
Expand All @@ -117,22 +120,20 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt

return nil, errors.New(err)
}
containerName = t.Spec.Containers[0].Name
} else {
containerName = opts.Container
currOpts.Container = t.Spec.Containers[0].Name
}

container, fieldPath := podcmd.FindContainerByName(t, containerName)
container, fieldPath := podcmd.FindContainerByName(t, currOpts.Container)
if container == nil {
return nil, fmt.Errorf("container %s is not valid for pod %s", opts.Container, t.Name)
return nil, fmt.Errorf("container %s is not valid for pod %s", currOpts.Container, t.Name)
}
ref, err := reference.GetPartialReference(scheme.Scheme, t, fieldPath)
if err != nil {
return nil, fmt.Errorf("Unable to construct reference to '%#v': %v", t, err)
}

ret := make(map[corev1.ObjectReference]rest.ResponseWrapper, 1)
ret[*ref] = clientset.Pods(t.Namespace).GetLogs(t.Name, opts)
ret[*ref] = clientset.Pods(t.Namespace).GetLogs(t.Name, currOpts)
return ret, nil
}

Expand Down
78 changes: 71 additions & 7 deletions staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestLogsForObject(t *testing.T) {
name: "pod logs",
obj: testPodWithOneContainers(),
actions: []testclient.Action{
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{
{
Expand Down Expand Up @@ -129,7 +129,7 @@ func TestLogsForObject(t *testing.T) {
Items: []corev1.Pod{*testPodWithOneContainers()},
},
actions: []testclient.Action{
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand All @@ -139,6 +139,70 @@ func TestLogsForObject(t *testing.T) {
FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name),
}},
},
{
name: "pods list logs: default container should not leak across pods",
obj: &corev1.PodList{
Items: []corev1.Pod{
{
TypeMeta: metav1.TypeMeta{
Kind: "pod",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "test",
Labels: map[string]string{"test": "logs"},
Annotations: map[string]string{
"kubectl.kubernetes.io/default-container": "c1",
},
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyAlways,
DNSPolicy: corev1.DNSClusterFirst,
Containers: []corev1.Container{
{Name: "c1"},
{Name: "c2"},
},
},
},
{
TypeMeta: metav1.TypeMeta{
Kind: "pod",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "test",
Labels: map[string]string{"test": "logs"},
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyAlways,
DNSPolicy: corev1.DNSClusterFirst,
Containers: []corev1.Container{
{Name: "c2"},
},
},
},
},
},
actions: []testclient.Action{
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
getLogsAction("test", &corev1.PodLogOptions{Container: "c2"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: "pod",
APIVersion: "v1",
Name: "foo",
Namespace: "test",
FieldPath: fmt.Sprintf("spec.containers{%s}", "c1"),
}, {
Kind: "pod",
APIVersion: "v1",
Name: "bar",
Namespace: "test",
FieldPath: fmt.Sprintf("spec.containers{%s}", "c2"),
}},
},
{
name: "pods list logs: all containers",
obj: &corev1.PodList{
Expand Down Expand Up @@ -201,7 +265,7 @@ func TestLogsForObject(t *testing.T) {
clientsetPods: []runtime.Object{testPodWithOneContainers()},
actions: []testclient.Action{
testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}),
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand All @@ -222,7 +286,7 @@ func TestLogsForObject(t *testing.T) {
clientsetPods: []runtime.Object{testPodWithOneContainers()},
actions: []testclient.Action{
testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}),
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand All @@ -243,7 +307,7 @@ func TestLogsForObject(t *testing.T) {
clientsetPods: []runtime.Object{testPodWithOneContainers()},
actions: []testclient.Action{
testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}),
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand All @@ -264,7 +328,7 @@ func TestLogsForObject(t *testing.T) {
clientsetPods: []runtime.Object{testPodWithOneContainers()},
actions: []testclient.Action{
testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}),
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand All @@ -285,7 +349,7 @@ func TestLogsForObject(t *testing.T) {
clientsetPods: []runtime.Object{testPodWithOneContainers()},
actions: []testclient.Action{
testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}),
getLogsAction("test", nil),
getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}),
},
expectedSources: []corev1.ObjectReference{{
Kind: testPodWithOneContainers().Kind,
Expand Down

0 comments on commit d7686a8

Please sign in to comment.