From f01d88487562baea3213bafa064bb21e1abce224 Mon Sep 17 00:00:00 2001 From: Sunny Date: Wed, 6 Sep 2023 14:19:46 +0000 Subject: [PATCH 1/2] oci: Use controller-runtime pkg/log explicitly This helps avoid importing the controller-runtime pkg/client/config package which has a flag initialization for "kubeconfig". This results in all the users of the oci package to also have the flag initialized in their applications. The usage of controller-runtime in oci package is just for logging. Importing the pkg/log package specifically helps avoid importing the client config which sets the flag. Signed-off-by: Sunny --- oci/auth/aws/auth.go | 4 ++-- oci/auth/azure/auth.go | 8 ++++---- oci/auth/gcp/auth.go | 8 ++++---- oci/auth/login/login.go | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/oci/auth/aws/auth.go b/oci/auth/aws/auth.go index fbaa208e..1019a86a 100644 --- a/oci/auth/aws/auth.go +++ b/oci/auth/aws/auth.go @@ -29,7 +29,7 @@ import ( "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/ecr" "github.com/google/go-containerregistry/pkg/authn" - ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/fluxcd/pkg/oci" ) @@ -135,7 +135,7 @@ func (c *Client) getLoginAuth(ctx context.Context, awsEcrRegion string) (authn.A // Login attempts to get the authentication material for ECR. func (c *Client) Login(ctx context.Context, autoLogin bool, image string) (authn.Authenticator, error) { if autoLogin { - ctrl.LoggerFrom(ctx).Info("logging in to AWS ECR for " + image) + log.FromContext(ctx).Info("logging in to AWS ECR for " + image) _, awsEcrRegion, ok := ParseRegistry(image) if !ok { return nil, errors.New("failed to parse AWS ECR image, invalid ECR image") diff --git a/oci/auth/azure/auth.go b/oci/auth/azure/auth.go index 3f797528..eea68d82 100644 --- a/oci/auth/azure/auth.go +++ b/oci/auth/azure/auth.go @@ -28,7 +28,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/fluxcd/pkg/oci" ) @@ -126,13 +126,13 @@ func ValidHost(host string) bool { // ensure that the passed image is a valid ACR image using ValidHost(). func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, error) { if autoLogin { - ctrl.LoggerFrom(ctx).Info("logging in to Azure ACR for " + image) + log.FromContext(ctx).Info("logging in to Azure ACR for " + image) // get registry host from image strArr := strings.SplitN(image, "/", 2) endpoint := fmt.Sprintf("%s://%s", c.scheme, strArr[0]) authConfig, err := c.getLoginAuth(ctx, endpoint) if err != nil { - ctrl.LoggerFrom(ctx).Info("error logging into ACR " + err.Error()) + log.FromContext(ctx).Info("error logging into ACR " + err.Error()) return nil, err } @@ -149,7 +149,7 @@ func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref na func (c *Client) OIDCLogin(ctx context.Context, registryUrl string) (authn.Authenticator, error) { authConfig, err := c.getLoginAuth(ctx, registryUrl) if err != nil { - ctrl.LoggerFrom(ctx).Info("error logging into ACR " + err.Error()) + log.FromContext(ctx).Info("error logging into ACR " + err.Error()) return nil, err } diff --git a/oci/auth/gcp/auth.go b/oci/auth/gcp/auth.go index 52bed87d..2808bbb6 100644 --- a/oci/auth/gcp/auth.go +++ b/oci/auth/gcp/auth.go @@ -26,7 +26,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/fluxcd/pkg/oci" ) @@ -105,10 +105,10 @@ func (c *Client) getLoginAuth(ctx context.Context) (authn.AuthConfig, error) { // ensure that the passed image is a valid GCR image using ValidHost(). func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref name.Reference) (authn.Authenticator, error) { if autoLogin { - ctrl.LoggerFrom(ctx).Info("logging in to GCP GCR for " + image) + log.FromContext(ctx).Info("logging in to GCP GCR for " + image) authConfig, err := c.getLoginAuth(ctx) if err != nil { - ctrl.LoggerFrom(ctx).Info("error logging into GCP " + err.Error()) + log.FromContext(ctx).Info("error logging into GCP " + err.Error()) return nil, err } @@ -122,7 +122,7 @@ func (c *Client) Login(ctx context.Context, autoLogin bool, image string, ref na func (c *Client) OIDCLogin(ctx context.Context) (authn.Authenticator, error) { authConfig, err := c.getLoginAuth(ctx) if err != nil { - ctrl.LoggerFrom(ctx).Info("error logging into GCP " + err.Error()) + log.FromContext(ctx).Info("error logging into GCP " + err.Error()) return nil, err } diff --git a/oci/auth/login/login.go b/oci/auth/login/login.go index a93aa7e2..b56bb462 100644 --- a/oci/auth/login/login.go +++ b/oci/auth/login/login.go @@ -24,7 +24,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/log" "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/oci/auth/aws" @@ -140,19 +140,19 @@ func (m *Manager) OIDCLogin(ctx context.Context, registryURL string, opts Provid if !opts.AwsAutoLogin { return nil, fmt.Errorf("ECR authentication failed: %w", oci.ErrUnconfiguredProvider) } - ctrl.LoggerFrom(ctx).Info("logging in to AWS ECR for " + u.Host) + log.FromContext(ctx).Info("logging in to AWS ECR for " + u.Host) return m.ecr.OIDCLogin(ctx, u.Host) case oci.ProviderGCP: if !opts.GcpAutoLogin { return nil, fmt.Errorf("GCR authentication failed: %w", oci.ErrUnconfiguredProvider) } - ctrl.LoggerFrom(ctx).Info("logging in to GCP GCR for " + u.Host) + log.FromContext(ctx).Info("logging in to GCP GCR for " + u.Host) return m.gcr.OIDCLogin(ctx) case oci.ProviderAzure: if !opts.AzureAutoLogin { return nil, fmt.Errorf("ACR authentication failed: %w", oci.ErrUnconfiguredProvider) } - ctrl.LoggerFrom(ctx).Info("logging in to Azure ACR for " + u.Host) + log.FromContext(ctx).Info("logging in to Azure ACR for " + u.Host) return m.acr.OIDCLogin(ctx, fmt.Sprintf("%s://%s", u.Scheme, u.Host)) } return nil, nil From e6669d82c8635a247b1034ee7d74e17229eeb5a7 Mon Sep 17 00:00:00 2001 From: Sunny Date: Wed, 6 Sep 2023 16:01:36 +0000 Subject: [PATCH 2/2] oci/auth: Add test to check for non-test flags Add a black box test to import the auth package as a consumer of the package and make sure that no flags are injected. Being in a test, it ignores all the default test flags with "test." prefix. Signed-off-by: Sunny --- oci/auth/flag_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 oci/auth/flag_test.go diff --git a/oci/auth/flag_test.go b/oci/auth/flag_test.go new file mode 100644 index 00000000..229ba8f7 --- /dev/null +++ b/oci/auth/flag_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Make sure we never inject non-test flags when the auth packages are imported. +// Refer https://github.com/fluxcd/pkg/issues/645. +package auth_test + +import ( + "flag" + "strings" + "testing" + + _ "github.com/fluxcd/pkg/oci/auth/login" +) + +func TestNonTestFlagCheck(t *testing.T) { + flagCheck := func(f *flag.Flag) { + if !strings.HasPrefix(f.Name, "test.") { + t.Errorf("found non-test command line flag: %q", f.Name) + } + } + flag.VisitAll(flagCheck) +}