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

Feat: Support SSH for retrieving terraform modules in private git repo #5059

Merged
merged 13 commits into from
Dec 23, 2022

Conversation

motilayo
Copy link
Contributor

@motilayo motilayo commented Nov 14, 2022

Support SSH for retrieving terraform modules in private git repo

Enable support for terraform modules in private git repo through SSH. A kubernetes secret which holds the ssh private-key and known_hosts should be created and referenced in the component definition.

Terraform controller PR: kubevela/terraform-controller#349

Feature for kubevela/terraform-controller#292

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Tested in a local cluster.

  1. Create a kubernetes secret of type kubernetes.io/ssh-auth for the git ssh credentials & known-hosts
apiVersion: v1
kind: Secret
metadata:
  name: git-ssh
  namespace: vela-system
type: kubernetes.io/ssh-auth
stringData:
  ssh-privatekey: |
   <SSH Private Key> # the ssh private key used for authenticating git
  known_hosts: |
   <SSH known_hosts>  # use `ssh-keyscan github.com`  to generate known_hosts
  1. Create a component definition
apiVersion: core.oam.dev/v1beta1
kind: ComponentDefinition
metadata:
  annotations:
    definition.oam.dev/description: Terraform module to provision a KMS key with alias
  labels:
    type: terraform
  name: aws-kms-key
  namespace: vela-system
spec:
  schematic:
    terraform:
      configuration: git@github.com:motilayo/terraform-aws-kms-key.git # use terraform configuration in private git repo to test
      gitCredentialsSecretReference: # reference to the secret created earlier
        name: git-ssh # secret name
        namespace: vela-system  # secret namespace
      type: remote
  1. Create an application and verify

Special notes for your reviewer

@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Base: 49.60% // Head: 61.08% // Increases project coverage by +11.47% 🎉

Coverage data is based on head (8785823) compared to base (c7c6009).
Patch coverage: 71.42% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #5059       +/-   ##
===========================================
+ Coverage   49.60%   61.08%   +11.47%     
===========================================
  Files         304      305        +1     
  Lines       45429    45499       +70     
===========================================
+ Hits        22537    27792     +5255     
+ Misses      20544    14846     -5698     
- Partials     2348     2861      +513     
Flag Coverage Δ
apiserver-e2etests 35.05% <0.00%> (?)
apiserver-unittests 36.84% <ø> (-0.05%) ⬇️
core-unittests 55.12% <62.50%> (-0.02%) ⬇️
e2e-multicluster-test 18.87% <14.28%> (-0.03%) ⬇️
e2e-rollout-tests 20.53% <16.07%> (+0.01%) ⬆️
e2etests 25.91% <16.07%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/controller/utils/capability.go 80.64% <68.62%> (-0.57%) ⬇️
pkg/appfile/appfile.go 74.31% <100.00%> (+0.16%) ⬆️
...ponentdefinition/componentdefinition_controller.go 91.35% <100.00%> (ø)
pkg/addon/registry.go 38.65% <0.00%> (-5.05%) ⬇️
pkg/cue/definition/template.go 64.16% <0.00%> (-3.47%) ⬇️
...troller/core.oam.dev/v1alpha2/application/apply.go 87.84% <0.00%> (-2.09%) ⬇️
pkg/oam/util/helper.go 61.91% <0.00%> (-0.53%) ⬇️
pkg/apiserver/utils/container/container.go 83.33% <0.00%> (ø)
pkg/velaql/providers/query/tree.go 85.27% <0.00%> (+0.26%) ⬆️
pkg/multicluster/cluster_management.go 36.70% <0.00%> (+0.26%) ⬆️
... and 113 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

motilayo and others added 9 commits December 21, 2022 22:50
Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>
Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>
Signed-off-by: motilayo <joshuaagboola@live.ca>
…date secret, improve known_hosts logic

Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
Signed-off-by: motilayo <joshuaagboola@live.ca>
@motilayo
Copy link
Contributor Author

@wonderflow Please review when you get a chance. I've added some tests and rebased my branch with the master branch.

go.mod Outdated Show resolved Hide resolved
Copy link
Collaborator

@Somefive Somefive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after nit.

go.mod Outdated Show resolved Hide resolved
pkg/controller/utils/capability.go Outdated Show resolved Hide resolved
pkg/controller/utils/capability.go Outdated Show resolved Hide resolved
@@ -724,6 +724,10 @@ func generateTerraformConfigurationWorkload(wl *Workload, ns string) (*unstructu
configuration.Spec.ProviderReference = wl.FullTemplate.ComponentDefinition.Spec.Schematic.Terraform.ProviderReference
}

if configuration.Spec.GitCredentialsSecretReference == nil {
configuration.Spec.GitCredentialsSecretReference = wl.FullTemplate.ComponentDefinition.Spec.Schematic.Terraform.GitCredentialsSecretReference
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the value is still nil?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it's ok to have no credentials. Like when it is a public repo.

Copy link
Collaborator

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job, generally LGTM!

Signed-off-by: motilayo <joshuaagboola@live.ca>
Copy link
Member

@chivalryq chivalryq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Please fix tests.

pkg/controller/utils/capability_test.go Show resolved Hide resolved
Signed-off-by: motilayo <joshuaagboola@live.ca>
@wonderflow wonderflow merged commit a9bc43a into kubevela:master Dec 23, 2022
barnettZQG pushed a commit to barnettZQG/kubevela that referenced this pull request Jan 30, 2023
kubevela#5059)

* Feat: Support SSH for retrieving terraform modules in private git repo

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>

* fix lint errors

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>

* fix gofmt lint error

Signed-off-by: motilayo <joshuaagboola@live.ca>

* update gitCredentialsReference to gitCredentialsSecretReference, validate secret, improve known_hosts logic

Signed-off-by: motilayo <joshuaagboola@live.ca>

* SImplify logic to get publickey

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Set known_hosts and export SSH_KNOWN_HOSTS

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Call GetGitSSHPublicKey for cli

Signed-off-by: motilayo <joshuaagboola@live.ca>

* fix parser.go - nil check for ref.Client

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Unit test for GetGitSSHPublicKey

Signed-off-by: motilayo <joshuaagboola@live.ca>

* improve test coverage

Signed-off-by: motilayo <joshuaagboola@live.ca>

* make reviewable

Signed-off-by: motilayo <joshuaagboola@live.ca>

* minor improvements & cleanup

Signed-off-by: motilayo <joshuaagboola@live.ca>

* update secret name in test

Signed-off-by: motilayo <joshuaagboola@live.ca>

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>
Signed-off-by: motilayo <joshuaagboola@live.ca>
zhaohuiweixiao pushed a commit to zhaohuiweixiao/kubevela that referenced this pull request Mar 7, 2023
kubevela#5059)

* Feat: Support SSH for retrieving terraform modules in private git repo

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>

* fix lint errors

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>

* fix gofmt lint error

Signed-off-by: motilayo <joshuaagboola@live.ca>

* update gitCredentialsReference to gitCredentialsSecretReference, validate secret, improve known_hosts logic

Signed-off-by: motilayo <joshuaagboola@live.ca>

* SImplify logic to get publickey

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Set known_hosts and export SSH_KNOWN_HOSTS

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Call GetGitSSHPublicKey for cli

Signed-off-by: motilayo <joshuaagboola@live.ca>

* fix parser.go - nil check for ref.Client

Signed-off-by: motilayo <joshuaagboola@live.ca>

* Unit test for GetGitSSHPublicKey

Signed-off-by: motilayo <joshuaagboola@live.ca>

* improve test coverage

Signed-off-by: motilayo <joshuaagboola@live.ca>

* make reviewable

Signed-off-by: motilayo <joshuaagboola@live.ca>

* minor improvements & cleanup

Signed-off-by: motilayo <joshuaagboola@live.ca>

* update secret name in test

Signed-off-by: motilayo <joshuaagboola@live.ca>

Signed-off-by: motilayo <44736801+motilayo@users.noreply.github.com>
Signed-off-by: motilayo <joshuaagboola@live.ca>
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.

4 participants