[credential provider] Add a flag mirrorMapping #6846

lzhecheng · 2024-08-19T10:55:36Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

[credential provider] Add a flag mirrorMapping. This flag is to mirror registry A to B when fetching credential.

Which issue(s) this PR fixes:

Fixes #6845

Special notes for your reviewer:

Does this PR introduce a user-facing change?

[credential provider] Add a flag mirrorMapping. This flag is to mirror registry A to B when fetching credential.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

examples/out-of-tree/credential-provider-config.yaml

bingosummer · 2024-08-27T02:18:27Z

pkg/credentialprovider/azure_credentials.go

+	return image, ""
+}
+
+// processMirrorMapping input format: "--mirror-mapping=aaa:bbb,ccc:ddd"


how to handle "aaa:bbb,aaa:ccc"?

Current logic takes aaa:ccc

so this PR only supports one mirror mapping?

No, it supports multiple.
Bin's question is how to handle two mappings with the same source registry. The logic is that the latter one takes effect.

pkg/credentialprovider/azure_credentials.go

andyzhangx

pls also add detailed description in the PR for mirror-mapping=mcr.microsoft.com:xxx.azurecr.io mapping

andyzhangx · 2024-08-30T04:25:20Z

cmd/acr-credential-provider/main.go

@@ -60,6 +63,9 @@ func main() {
 	logs.InitLogs()
 	defer logs.FlushLogs()

+	// Flags
+	command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to use")


this flag value is defined after it's used in L50, then how could external user use defined flag to override this?

I think as long as it is before command.Execute() then it is ok.

Please check the CCM code. You can see cmd is defined first, then flags.

cloud-provider-azure/cmd/cloud-controller-manager/app/controllermanager.go

Lines 79 to 159 in 9eb8173

cmd := &cobra.Command{

Use: "cloud-controller-manager",

Long: `The Cloud controller manager is a daemon that embeds the cloud specific control loops shipped with Kubernetes.`,

Run: func(cmd *cobra.Command, _ []string) {

verflag.PrintAndExitIfRequested("Cloud Provider Azure")

cliflag.PrintFlags(cmd.Flags())

c, err := s.Config(KnownControllers(), ControllersDisabledByDefault.List(), controllerAliases)

if err != nil {

klog.Errorf("Run: failed to configure cloud controller manager: %v", err)

os.Exit(1)

}

healthHandler, err := StartHTTPServer(c.Complete(), wait.NeverStop)

if err != nil {

klog.Errorf("Run: railed to start HTTP server: %v", err)

os.Exit(1)

}

if c.ComponentConfig.Generic.LeaderElection.LeaderElect {

// Identity used to distinguish between multiple cloud controller manager instances

id, err := os.Hostname()

if err != nil {

klog.Errorf("Run: failed to get host name: %v", err)

os.Exit(1)

}

// add a uniquifier so that two processes on the same host don't accidentally both become active

id = id + "_" + string(uuid.NewUUID())

// Lock required for leader election

rl, err := resourcelock.NewFromKubeconfig(c.ComponentConfig.Generic.LeaderElection.ResourceLock,

c.ComponentConfig.Generic.LeaderElection.ResourceNamespace,

c.ComponentConfig.Generic.LeaderElection.ResourceName,

resourcelock.ResourceLockConfig{

Identity: id,

EventRecorder: c.EventRecorder,

},

c.Kubeconfig,

c.ComponentConfig.Generic.LeaderElection.RenewDeadline.Duration)

if err != nil {

klog.Fatalf("error creating lock: %v", err)

}

// Try and become the leader and start cloud controller manager loops

var electionChecker *leaderelection.HealthzAdaptor

if c.ComponentConfig.Generic.LeaderElection.LeaderElect {

electionChecker = leaderelection.NewLeaderHealthzAdaptor(time.Second * 20)

}

leaderelection.RunOrDie(context.TODO(), leaderelection.LeaderElectionConfig{

Lock: rl,

LeaseDuration: c.ComponentConfig.Generic.LeaderElection.LeaseDuration.Duration,

RenewDeadline: c.ComponentConfig.Generic.LeaderElection.RenewDeadline.Duration,

RetryPeriod: c.ComponentConfig.Generic.LeaderElection.RetryPeriod.Duration,

Callbacks: leaderelection.LeaderCallbacks{

OnStartedLeading: RunWrapper(s, c, healthHandler),

OnStoppedLeading: func() {

klog.ErrorS(nil, "leaderelection lost")

klog.FlushAndExit(klog.ExitFlushTimeout, 1)

},

},

WatchDog: electionChecker,

Name: "cloud-controller-manager",

})

panic("unreachable")

}

RunWrapper(s, c, healthHandler)(context.TODO())

panic("unreachable")

},

}

fs := cmd.Flags()

namedFlagSets := s.Flags(KnownControllers(), ControllersDisabledByDefault.List())

verflag.AddFlags(namedFlagSets.FlagSet("global"))

globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())

for _, f := range namedFlagSets.FlagSets {

fs.AddFlagSet(f)

}

andyzhangx · 2024-08-30T04:27:00Z

pkg/credentialprovider/azure_credentials.go

+	return image, ""
+}
+
+// processMirrorMapping input format: "--mirror-mapping=aaa:bbb,ccc:ddd"


so this PR only supports one mirror mapping?

andyzhangx · 2024-08-30T04:30:55Z

pkg/credentialprovider/azure_credentials.go

 			Username: username,
 			Password: password,
 		}
+		if sourceloginServer != "" {
+			response.Auth[sourceloginServer] = v1.AuthConfig{


you will add auth for both sourceloginServer and targetloginServer, is that necessary?

Yes. I think it does no harm, right.

then you will use password pull the mcr image, not sure whether that has side effect, in this case, only targetloginServer needs password?

When mirror mapping is used, in this case, sourceLoginServer is like MCR and targetloginServer is like ACR. So, it is the sourceloginServer needs password.

mcr + acr auth is a MUST, additional auth should bring no hard.
kubelet will send mcr image + acr auth to containerd to pull mcr image, and containerd will mirror the mcr image to acr through registry host, and finally pull mcr image from acr server.

pkg/credentialprovider/azure_credentials.go

examples/out-of-tree/credential-provider-config.yaml

mainred · 2024-09-03T07:16:22Z

pkg/credentialprovider/azure_credentials.go

 			Username: username,
 			Password: password,
 		}
+		if sourceloginServer != "" {
+			response.Auth[sourceloginServer] = v1.AuthConfig{


mcr + acr auth is a MUST, additional auth should bring no hard.
kubelet will send mcr image + acr auth to containerd to pull mcr image, and containerd will mirror the mcr image to acr through registry host, and finally pull mcr image from acr server.

mainred · 2024-09-03T07:17:47Z

pkg/credentialprovider/azure_credentials.go

+
+// processMirrorMapping input format: "--mirror-mapping=aaa:bbb,ccc:ddd"
+// output format: map[string]string{"aaa": "bbb", "ccc": "ddd"}
+func processMirrorMapping(mirrorMappingStr string) map[string]string {


nit, Maybe parseMirrorMapping to be specific, and to not conflict with other processXXX func.

mainred · 2024-09-03T07:25:19Z

cmd/acr-credential-provider/main.go

@@ -60,6 +63,9 @@ func main() {
 	logs.InitLogs()
 	defer logs.FlushLogs()

+	// Flags
+	command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to use")


Please let's elaborate the flags. I did not see the detailed description of this flag anywhere else.

Suggested change

command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to use")

command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to mirror a source registry host to a target registry host, and image pull credentail will be requested to the target registry host when the image is from source registry host")

The name may be registry-mirror, since mirror is also the term used in containerd host namespace feature https://github.com/containerd/containerd/blob/main/docs/hosts.md#registry-host-namespace

With registry mirror, e.g. --registry-mirror=mcr.microsoft.com:xxx.azurecr.io credential provider will return credentials of xxx.azurecr.io for mcr.microsoft.com. In this way, an image with URL prefix mcr.microsoft.com, but actually in xxx.azurecr.io, can be successfully pulled. Signed-off-by: Zhecheng Li <zhechengli@microsoft.com>

coveralls · 2024-09-09T02:08:25Z

coverage: 77.247% (+0.01%) from 77.233%
when pulling b12bb67 on lzhecheng:credential-provider-mirror-mapping
into 6d2a767 on kubernetes-sigs:master.

lzhecheng · 2024-09-10T01:58:14Z

Verified locally. SP and MSI work.

andyzhangx

/lgtm

k8s-ci-robot · 2024-09-10T01:59:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, lzhecheng

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:

~~OWNERS~~ [andyzhangx,lzhecheng]

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

lzhecheng · 2024-10-16T08:05:04Z

/cherrypick release-1.31

lzhecheng · 2024-10-16T08:05:09Z

/cherrypick release-1.30

k8s-infra-cherrypick-robot · 2024-10-16T08:05:50Z

@lzhecheng: new pull request created: #7335

In response to this:

/cherrypick release-1.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-infra-cherrypick-robot · 2024-10-16T08:05:51Z

@lzhecheng: new pull request created: #7336

In response to this:

/cherrypick release-1.30

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 19, 2024

k8s-ci-robot requested review from feiskyer and nilo19 August 19, 2024 10:55

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 19, 2024

bingosummer reviewed Aug 27, 2024

View reviewed changes

lzhecheng force-pushed the credential-provider-mirror-mapping branch from 8c0a9e2 to 3d563f5 Compare August 28, 2024 08:50

andyzhangx reviewed Aug 30, 2024

View reviewed changes

lzhecheng force-pushed the credential-provider-mirror-mapping branch from 3d563f5 to 72952fe Compare August 30, 2024 10:43

mainred reviewed Sep 3, 2024

View reviewed changes

lzhecheng force-pushed the credential-provider-mirror-mapping branch from 72952fe to ec05532 Compare September 9, 2024 01:51

lzhecheng force-pushed the credential-provider-mirror-mapping branch from ec05532 to b12bb67 Compare September 9, 2024 01:54

andyzhangx approved these changes Sep 10, 2024

View reviewed changes

k8s-ci-robot assigned andyzhangx Sep 10, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 10, 2024

k8s-ci-robot merged commit 220c71c into kubernetes-sigs:master Sep 10, 2024
18 checks passed

lzhecheng deleted the credential-provider-mirror-mapping branch September 10, 2024 13:18

This was referenced Oct 16, 2024

[release-1.31] [credential provider] Add a flag mirrorMapping #7335

Merged

[release-1.30] [credential provider] Add a flag mirrorMapping #7336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[credential provider] Add a flag mirrorMapping #6846

[credential provider] Add a flag mirrorMapping #6846

lzhecheng commented Aug 19, 2024

bingosummer Aug 27, 2024

lzhecheng Aug 28, 2024

andyzhangx Aug 30, 2024

lzhecheng Aug 30, 2024

andyzhangx left a comment

andyzhangx Aug 30, 2024

lzhecheng Aug 30, 2024

andyzhangx Aug 30, 2024

andyzhangx Aug 30, 2024

lzhecheng Aug 30, 2024

andyzhangx Aug 30, 2024

lzhecheng Aug 30, 2024

mainred Sep 3, 2024

mainred Sep 3, 2024

mainred Sep 3, 2024 •

edited

Loading

lzhecheng Sep 9, 2024

mainred Sep 3, 2024

mainred Sep 3, 2024

lzhecheng Sep 9, 2024

coveralls commented Sep 9, 2024 •

edited

Loading

lzhecheng commented Sep 10, 2024

andyzhangx left a comment

k8s-ci-robot commented Sep 10, 2024

lzhecheng commented Oct 16, 2024

lzhecheng commented Oct 16, 2024

k8s-infra-cherrypick-robot commented Oct 16, 2024

k8s-infra-cherrypick-robot commented Oct 16, 2024

	cmd := &cobra.Command{
	Use: "cloud-controller-manager",
	Long: `The Cloud controller manager is a daemon that embeds the cloud specific control loops shipped with Kubernetes.`,
	Run: func(cmd *cobra.Command, _ []string) {
	verflag.PrintAndExitIfRequested("Cloud Provider Azure")
	cliflag.PrintFlags(cmd.Flags())

	c, err := s.Config(KnownControllers(), ControllersDisabledByDefault.List(), controllerAliases)
	if err != nil {
	klog.Errorf("Run: failed to configure cloud controller manager: %v", err)
	os.Exit(1)
	}

	healthHandler, err := StartHTTPServer(c.Complete(), wait.NeverStop)
	if err != nil {
	klog.Errorf("Run: railed to start HTTP server: %v", err)
	os.Exit(1)
	}

	if c.ComponentConfig.Generic.LeaderElection.LeaderElect {
	// Identity used to distinguish between multiple cloud controller manager instances
	id, err := os.Hostname()
	if err != nil {
	klog.Errorf("Run: failed to get host name: %v", err)
	os.Exit(1)
	}
	// add a uniquifier so that two processes on the same host don't accidentally both become active
	id = id + "_" + string(uuid.NewUUID())

	// Lock required for leader election
	rl, err := resourcelock.NewFromKubeconfig(c.ComponentConfig.Generic.LeaderElection.ResourceLock,
	c.ComponentConfig.Generic.LeaderElection.ResourceNamespace,
	c.ComponentConfig.Generic.LeaderElection.ResourceName,
	resourcelock.ResourceLockConfig{
	Identity: id,
	EventRecorder: c.EventRecorder,
	},
	c.Kubeconfig,
	c.ComponentConfig.Generic.LeaderElection.RenewDeadline.Duration)
	if err != nil {
	klog.Fatalf("error creating lock: %v", err)
	}

	// Try and become the leader and start cloud controller manager loops
	var electionChecker *leaderelection.HealthzAdaptor
	if c.ComponentConfig.Generic.LeaderElection.LeaderElect {
	electionChecker = leaderelection.NewLeaderHealthzAdaptor(time.Second * 20)
	}

	leaderelection.RunOrDie(context.TODO(), leaderelection.LeaderElectionConfig{
	Lock: rl,
	LeaseDuration: c.ComponentConfig.Generic.LeaderElection.LeaseDuration.Duration,
	RenewDeadline: c.ComponentConfig.Generic.LeaderElection.RenewDeadline.Duration,
	RetryPeriod: c.ComponentConfig.Generic.LeaderElection.RetryPeriod.Duration,
	Callbacks: leaderelection.LeaderCallbacks{
	OnStartedLeading: RunWrapper(s, c, healthHandler),
	OnStoppedLeading: func() {
	klog.ErrorS(nil, "leaderelection lost")
	klog.FlushAndExit(klog.ExitFlushTimeout, 1)
	},
	},
	WatchDog: electionChecker,
	Name: "cloud-controller-manager",
	})

	panic("unreachable")
	}

	RunWrapper(s, c, healthHandler)(context.TODO())
	panic("unreachable")
	},
	}

	fs := cmd.Flags()
	namedFlagSets := s.Flags(KnownControllers(), ControllersDisabledByDefault.List())
	verflag.AddFlags(namedFlagSets.FlagSet("global"))
	globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name())

	for _, f := range namedFlagSets.FlagSets {
	fs.AddFlagSet(f)
	}

	command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to use")
	command.Flags().StringVarP(&mirrorMappingStr, "mirror-mapping", "m", "", "mirror mapping to mirror a source registry host to a target registry host, and image pull credentail will be requested to the target registry host when the image is from source registry host")

[credential provider] Add a flag mirrorMapping #6846

[credential provider] Add a flag mirrorMapping #6846

Conversation

lzhecheng commented Aug 19, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyzhangx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mainred Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 9, 2024 • edited Loading

lzhecheng commented Sep 10, 2024

andyzhangx left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 10, 2024

lzhecheng commented Oct 16, 2024

lzhecheng commented Oct 16, 2024

k8s-infra-cherrypick-robot commented Oct 16, 2024

k8s-infra-cherrypick-robot commented Oct 16, 2024

mainred Sep 3, 2024 •

edited

Loading

coveralls commented Sep 9, 2024 •

edited

Loading