Skip to content

Commit

Permalink
Merge pull request #1221 from antechrestos/refactoring/default_transport
Browse files Browse the repository at this point in the history 
Remove direct use of DefaultTransport
  • Loading branch information
@tejal29
tejal29 authored May 20, 2020
2 parents 885c4da + 2f6090d commit d86e09b
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 247 deletions.
10 changes: 2 additions & 8 deletions pkg/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ limitations under the License.
package cache

import (
"crypto/tls"
"fmt"
"net/http"
"os"
"path"
"path/filepath"
"time"

"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/pkg/creds"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
Expand Down Expand Up @@ -67,12 +66,7 @@ func (rc *RegistryCache) RetrieveLayer(ck string) (v1.Image, error) {
cacheRef.Repository.Registry = newReg
}

tr := http.DefaultTransport.(*http.Transport)
if rc.Opts.SkipTLSVerifyRegistries.Contains(registryName) {
tr.TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,
}
}
tr := util.MakeTransport(rc.Opts, registryName)

img, err := remote.Image(cacheRef, remote.WithTransport(tr), remote.WithAuthFromKeychain(creds.GetKeychain()))
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions pkg/executor/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
image_util "github.com/GoogleContainerTools/kaniko/pkg/image"
"github.com/GoogleContainerTools/kaniko/pkg/snapshot"
"github.com/GoogleContainerTools/kaniko/pkg/timing"
"github.com/GoogleContainerTools/kaniko/pkg/util"
Expand Down Expand Up @@ -84,7 +85,7 @@ type stageBuilder struct {

// newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, crossStageDeps map[int][]string, dcm map[string]string, sid map[string]string, stageNameToIdx map[string]string) (*stageBuilder, error) {
sourceImage, err := util.RetrieveSourceImage(stage, opts)
sourceImage, err := image_util.RetrieveSourceImage(stage, opts)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -517,7 +518,7 @@ func CalculateDependencies(stages []config.KanikoStage, opts *config.KanikoOptio
} else if s.Name == constants.NoBaseImage {
image = empty.Image
} else {
image, err = util.RetrieveSourceImage(s, opts)
image, err = image_util.RetrieveSourceImage(s, opts)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -751,7 +752,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e

// This must be an image name, fetch it.
logrus.Debugf("Found extra base image stage %s", c.From)
sourceImage, err := util.RetrieveRemoteImage(c.From, opts)
sourceImage, err := image_util.RetrieveRemoteImage(c.From, opts)
if err != nil {
return err
}
Expand Down
70 changes: 7 additions & 63 deletions pkg/executor/push.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package executor

import (
"crypto/tls"
"crypto/x509"
"encoding/json"
"fmt"
"io/ioutil"
Expand All @@ -34,6 +32,7 @@ import (
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/creds"
"github.com/GoogleContainerTools/kaniko/pkg/timing"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/GoogleContainerTools/kaniko/pkg/version"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
Expand All @@ -48,15 +47,15 @@ import (
"github.com/spf13/afero"
)

type withUserAgent struct {
t http.RoundTripper
}

// for testing
var (
newRetry = transport.NewRetry
)

type withUserAgent struct {
t http.RoundTripper
}

const (
UpstreamClientUaKey = "UPSTREAM_CLIENT_TYPE"
)
Expand All @@ -82,41 +81,6 @@ func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) {
return w.t.RoundTrip(r)
}

type CertPool interface {
value() *x509.CertPool
append(path string) error
}

type X509CertPool struct {
inner x509.CertPool
}

func (p *X509CertPool) value() *x509.CertPool {
return &p.inner
}

func (p *X509CertPool) append(path string) error {
pem, err := ioutil.ReadFile(path)
if err != nil {
return err
}
p.inner.AppendCertsFromPEM(pem)
return nil
}

type systemCertLoader func() CertPool

var defaultX509Handler systemCertLoader = func() CertPool {
systemCertPool, err := x509.SystemCertPool()
if err != nil {
logrus.Warn("Failed to load system cert pool. Loading empty one instead.")
systemCertPool = x509.NewCertPool()
}
return &X509CertPool{
inner: *systemCertPool,
}
}

// for testing
var (
fs = afero.NewOsFs()
Expand Down Expand Up @@ -161,7 +125,7 @@ func CheckPushPermissions(opts *config.KanikoOptions) error {
}
destRef.Repository.Registry = newReg
}
tr := makeTransport(opts, registryName, defaultX509Handler)
tr := newRetry(util.MakeTransport(opts, registryName))
if err := checkRemotePushPermission(destRef, creds.GetKeychain(), tr); err != nil {
return errors.Wrapf(err, "checking push permission for %q", destRef)
}
Expand Down Expand Up @@ -258,7 +222,7 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
return errors.Wrap(err, "resolving pushAuth")
}

tr := makeTransport(opts, registryName, defaultX509Handler)
tr := newRetry(util.MakeTransport(opts, registryName))
rt := &withUserAgent{t: tr}

if err := remote.Write(destRef, image, remote.WithAuth(pushAuth), remote.WithTransport(rt)); err != nil {
Expand Down Expand Up @@ -300,26 +264,6 @@ func writeImageOutputs(image v1.Image, destRefs []name.Tag) error {
return nil
}

func makeTransport(opts *config.KanikoOptions, registryName string, loader systemCertLoader) http.RoundTripper {
// Create a transport to set our user-agent.
var tr http.RoundTripper = http.DefaultTransport.(*http.Transport).Clone()
if opts.SkipTLSVerify || opts.SkipTLSVerifyRegistries.Contains(registryName) {
tr.(*http.Transport).TLSClientConfig = &tls.Config{
InsecureSkipVerify: true,
}
} else if certificatePath := opts.RegistriesCertificates[registryName]; certificatePath != "" {
systemCertPool := loader()
if err := systemCertPool.append(certificatePath); err != nil {
logrus.WithError(err).Warnf("Failed to load certificate %s for %s\n", certificatePath, registryName)
} else {
tr.(*http.Transport).TLSClientConfig = &tls.Config{
RootCAs: systemCertPool.value(),
}
}
}
return newRetry(tr)
}

// pushLayerToCache pushes layer (tagged with cacheKey) to opts.Cache
// if opts.Cache doesn't exist, infer the cache from the given destination
func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, tarPath string, createdBy string) error {
Expand Down
100 changes: 0 additions & 100 deletions pkg/executor/push_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package executor

import (
"bytes"
"crypto/tls"
"crypto/x509"
"fmt"
"io/ioutil"
"net/http"
Expand All @@ -34,7 +32,6 @@ import (
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/layout"
"github.com/google/go-containerregistry/pkg/v1/random"
"github.com/google/go-containerregistry/pkg/v1/remote/transport"
"github.com/google/go-containerregistry/pkg/v1/validate"

"github.com/spf13/afero"
Expand Down Expand Up @@ -272,103 +269,6 @@ func TestImageNameDigestFile(t *testing.T) {

}

type mockedCertPool struct {
certificatesPath []string
}

func (m *mockedCertPool) value() *x509.CertPool {
return &x509.CertPool{}
}

func (m *mockedCertPool) append(path string) error {
m.certificatesPath = append(m.certificatesPath, path)
return nil
}

type retryTransport struct {
inner http.RoundTripper
}

func (t *retryTransport) RoundTrip(in *http.Request) (out *http.Response, err error) {
return nil, nil
}

func Test_makeTransport(t *testing.T) {
registryName := "my.registry.name"

tests := []struct {
name string
opts *config.KanikoOptions
check func(*tls.Config, *mockedCertPool)
}{
{
name: "SkipTLSVerify set",
opts: &config.KanikoOptions{SkipTLSVerify: true},
check: func(config *tls.Config, pool *mockedCertPool) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerify set")
}
},
},
{
name: "SkipTLSVerifyRegistries set with expected registry",
opts: &config.KanikoOptions{SkipTLSVerifyRegistries: []string{registryName}},
check: func(config *tls.Config, pool *mockedCertPool) {
if !config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify not set while SkipTLSVerifyRegistries set with registry name")
}
},
},
{
name: "SkipTLSVerifyRegistries set with other registry",
opts: &config.KanikoOptions{SkipTLSVerifyRegistries: []string{fmt.Sprintf("other.%s", registryName)}},
check: func(config *tls.Config, pool *mockedCertPool) {
if config.InsecureSkipVerify {
t.Errorf("makeTransport().TLSClientConfig.InsecureSkipVerify set while SkipTLSVerifyRegistries not set with registry name")
}
},
},
{
name: "RegistriesCertificates set for registry",
opts: &config.KanikoOptions{RegistriesCertificates: map[string]string{registryName: "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
if len(pool.certificatesPath) != 1 || pool.certificatesPath[0] != "/path/to/the/certificate.cert" {
t.Errorf("makeTransport().RegistriesCertificates certificate not appended to system certificates")
}
},
},
{
name: "RegistriesCertificates set for another registry",
opts: &config.KanikoOptions{RegistriesCertificates: map[string]string{fmt.Sprintf("other.%s=", registryName): "/path/to/the/certificate.cert"}},
check: func(config *tls.Config, pool *mockedCertPool) {
if len(pool.certificatesPath) != 0 {
t.Errorf("makeTransport().RegistriesCertificates certificate appended to system certificates while added for other registry")
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
originalRetry := newRetry
newRetry = func(inner http.RoundTripper, _ ...transport.Option) http.RoundTripper {
return &retryTransport{
inner: inner,
}
}
defer func() { newRetry = originalRetry }()
var certificatesPath []string
certPool := mockedCertPool{
certificatesPath: certificatesPath,
}
var mockedSystemCertLoader systemCertLoader = func() CertPool {
return &certPool
}
actual := makeTransport(tt.opts, registryName, mockedSystemCertLoader)
tt.check(actual.(*retryTransport).inner.(*http.Transport).TLSClientConfig, &certPool)
})
}
}

var calledExecCommand = false
var calledCheckPushPermission = false

Expand Down
43 changes: 0 additions & 43 deletions pkg/image/image.go
View file

This file was deleted.

Loading

0 comments on commit d86e09b

Please sign in to comment.