cloudviz / agentless-system-crawler

A tool to crawl systems like crawlers for the web
Apache License 2.0
116 stars 44 forks source link

fix icp env plugin to include containername again #364

Closed tatsuhirochiba closed 6 years ago

tatsuhirochiba commented 6 years ago

This PR Includes io.kubernetes.container.name again in namespace on icp environment, that is the same as kubernetes env plugin. Also fixed redundant code in sas-emitter and icp-environment plugins.

Signed-off-by: Tatsuhiro Chiba chiba@jp.ibm.com

sahilsuneja1 commented 6 years ago

@nadgowdas ?

tatsuhirochiba commented 6 years ago

I will update one more thing in icp env plugin..

tatsuhirochiba commented 6 years ago

@nadgowdas I simplified namespace creation logic for reg-crawler. Sorry for asking review again..

we used RepoTags info for making namespace of reg-crawling image, however, we can not retrieve an adequate RepoTag in the following case.

Two images are in the private repository. They are different name and tag, but the contents are same, so the image identity hash is also same.

mycluster.icp:8500/default/centos71                                    latest              2d194b392dd1        5 weeks ago         195MB
mycluster.icp:8500/kube-system/centos                                  centos7             2d194b392dd1        5 weeks ago         195MB

They also have same RepoTags too.

~# docker image inspect mycluster.icp:8500/default/centos71 | jq .[].RepoTags
[
  "mycluster.icp:8500/default/centos71:latest",
  "mycluster.icp:8500/kube-system/centos:centos7",
…
]

~# docker image inspect mycluster.icp:8500/kube-system/centos:centos7 | jq .[].RepoTags
[
  "mycluster.icp:8500/default/centos71:latest",
  "mycluster.icp:8500/kube-system/centos:centos7",
…
]

I assume that default/centos71:latest is the primary image in the private repo, which was pushed first. Then kube-system/centos:centos7 was pushed second.

In this case, there is no kube-system/centos:centos7 information in a launched container even if a container is based on kube-system/centos:centos7. Instead, the image name in the meta info of the container will be default/centos71:latest. That is because kube-system/centos:centos7 is translated into default/centos71:latest in the private repo.

So, past icp_env_plugin could not make an adequate namespace for the kube-system/centos:centos7.

This PR fixes the problem by appending --namespace crawler args in registry crawler level, and then icp env plugin loads the specified namespace from outside while crawling an image.

nadgowdas commented 6 years ago

@tatsuhirochiba I did not understood the problem above, specially following comment. That is because centos:centos7 is treated as centos71:latest. ?

tatsuhirochiba commented 6 years ago

@nadgowdas Sorry for confusing you. I updated the comments on the above to better understand.. The problem is repotag based namespace creation is failed in the above case, so I would like to pass a namespace from outside of crawler. (i.e. reg-crawler wrapper) Using --namespace is simplest way to pass specified namespace to the env plugin.

nadgowdas commented 6 years ago

Thanks @tatsuhirochiba for the explanation.

nadgowdas commented 6 years ago

@sahilsuneja1 do you have any comments or we can merge this.

sahilsuneja1 commented 6 years ago

Fine by me, merging.