Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

registry - generate a DOSecret operator friendly secret manifest #966

Conversation

varshavaradarajan
Copy link
Contributor

@varshavaradarajan varshavaradarajan commented Mar 8, 2021

The Kubernetes secret manifest we generate makes it easy for users to pull private images into their DOKS cluster. Now that we have the dosecret-operator to make the secret available in all namespaces, generate operator friendly manifests for users to apply into their DOKS cluster. Applying the manifest generated here to a DOKS cluster is the same as doing:

doctl kubernetes cluster registry add

cc @adamwg , @timoreimann

@varshavaradarajan varshavaradarajan force-pushed the varsha/make-manifest-dosecret-operator-friendly branch 2 times, most recently from d8f267f to 8097b50 Compare Mar 8, 2021
Copy link
Member

@andrewsomething andrewsomething left a comment

Looks like this integration test will need to be updated as well:

registryKubernetesManifestOutput = `
apiVersion: v1
data:
.dockerconfigjson: eyJhdXRocyI6eyJyZWdpc3RyeS5kaWdpdGFsb2NlYW4uY29tIjp7ImF1dGgiOiJZamRrTUROaE5qazBOMkl5TVRkbFptSTJaak5sWXpOaVpETTFNRFExT0RJNllqZGtNRE5oTmprME4ySXlNVGRsWm1JMlpqTmxZek5pWkRNMU1EUTFPRElLIn19fQ==
kind: Secret
metadata:
creationTimestamp: null
name: registry-my-registry
namespace: default
type: kubernetes.io/dockerconfigjson
`
)

Unfortunately, it doesn't seem to have run in this PR. I've identified and fixed the issue preventing it from running in: #967

@andrewsomething
Copy link
Member

@andrewsomething andrewsomething commented Mar 9, 2021

If you rebase on main, you'll see the failure:

=== CONT  TestRun/doctl/registry/kubernetes-manifest/prints_a_kubernetes_manifest_for_a_secret_with_the_registry_credentials
    registry_kubernetes_manifest_test.go:78: 
        	Error Trace:	registry_kubernetes_manifest_test.go:78
        	            				spec.go:269
        	            				spec.go:262
        	            				parser.go:133
        	Error:      	Not equal: 
        	            	expected: map[string]interface {}{"apiVersion":"v1", "data":map[string]interface {}{".dockerconfigjson":"eyJhdXRocyI6eyJyZWdpc3RyeS5kaWdpdGFsb2NlYW4uY29tIjp7ImF1dGgiOiJZamRrTUROaE5qazBOMkl5TVRkbFptSTJaak5sWXpOaVpETTFNRFExT0RJNllqZGtNRE5oTmprME4ySXlNVGRsWm1JMlpqTmxZek5pWkRNMU1EUTFPRElLIn19fQ=="}, "kind":"Secret", "metadata":map[string]interface {}{"creationTimestamp":interface {}(nil), "name":"registry-my-registry", "namespace":"default"}, "type":"kubernetes.io/dockerconfigjson"}
        	            	actual  : map[string]interface {}{"apiVersion":"v1", "data":map[string]interface {}{".dockerconfigjson":"eyJhdXRocyI6eyJyZWdpc3RyeS5kaWdpdGFsb2NlYW4uY29tIjp7ImF1dGgiOiJZamRrTUROaE5qazBOMkl5TVRkbFptSTJaak5sWXpOaVpETTFNRFExT0RJNllqZGtNRE5oTmprME4ySXlNVGRsWm1JMlpqTmxZek5pWkRNMU1EUTFPRElLIn19fQ=="}, "kind":"Secret", "metadata":map[string]interface {}{"annotations":map[string]interface {}{"digitalocean.com/dosecret-identifier":"registry-my-registry"}, "creationTimestamp":interface {}(nil), "name":"registry-my-registry", "namespace":"kube-system"}, "type":"kubernetes.io/dockerconfigjson"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -6,6 +6,9 @@
        	            	  (string) (len=4) "kind": (string) (len=6) "Secret",
        	            	- (string) (len=8) "metadata": (map[string]interface {}) (len=3) {
        	            	+ (string) (len=8) "metadata": (map[string]interface {}) (len=4) {
        	            	+  (string) (len=11) "annotations": (map[string]interface {}) (len=1) {
        	            	+   (string) (len=36) "digitalocean.com/dosecret-identifier": (string) (len=20) "registry-my-registry"
        	            	+  },
        	            	   (string) (len=17) "creationTimestamp": (interface {}) <nil>,
        	            	   (string) (len=4) "name": (string) (len=20) "registry-my-registry",
        	            	-  (string) (len=9) "namespace": (string) (len=7) "default"
        	            	+  (string) (len=9) "namespace": (string) (len=11) "kube-system"
        	            	  },
        	Test:       	TestRun/doctl/registry/kubernetes-manifest/prints_a_kubernetes_manifest_for_a_secret_with_the_registry_credentials

timoreimann
timoreimann previously approved these changes Mar 9, 2021
Copy link
Contributor

@timoreimann timoreimann left a comment

Grammar nit, otherwise looks good from a DOKS perspective (given it passes the integration tests that Andrew fixed).

commands/registry.go Outdated Show resolved Hide resolved
@varshavaradarajan varshavaradarajan force-pushed the varsha/make-manifest-dosecret-operator-friendly branch 2 times, most recently from 48c5c4e to 3be8cc6 Compare Mar 9, 2021
@andrewsomething andrewsomething self-requested a review Mar 9, 2021
andrewsomething
andrewsomething previously approved these changes Mar 9, 2021
Copy link
Member

@andrewsomething andrewsomething left a comment

👍 Looks good!

Question: Is the operator being back-filled onto existing clusters? Do we need any caveats about cluster versions in the help text?

@varshavaradarajan
Copy link
Contributor Author

@varshavaradarajan varshavaradarajan commented Mar 10, 2021

@andrewsomething - Good point! We have backfilled the operator until 1.15.12-do.2. Since this command is cluster-agnostic, what do you suggest we do? The manifest that this change generates for kube-system namespace or by default will trigger the operator in versions > 1.15.12-do.2. For unsupported versions, the manifest is harmless. For other namespaces, the manifest will be generated same as before.

@andrewsomething
Copy link
Member

@andrewsomething andrewsomething commented Mar 10, 2021

Maybe add something like this to the help for the command?

By default, the secret manifest will be applied to all the namespaces for the Kubernetes cluster using the DOSecret operator. The DOSecret operator is available on clusters running version 1.15.12-do.2 or greater. For older clusters or to restrict the secret to a specific namespace, use the --namespace flag.

@andrewsomething andrewsomething self-requested a review Mar 16, 2021
Copy link
Member

@andrewsomething andrewsomething left a comment

👍

@andrewsomething andrewsomething merged commit 552c32c into digitalocean:main Mar 16, 2021
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants