DRAFT control-service: AWS Code commit integration by SurajViitk · Pull Request #3304 · vmware/versatile-data-kit

SurajViitk · 2024-04-15T08:56:52Z

Just exposing AWS credentials retreived by git to test them for AWS codecommit

EDIT: removed this

vmwclabot · 2024-04-15T08:56:59Z

@SurajViitk, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

vmwclabot · 2024-04-15T09:03:18Z

@SurajViitk, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

for more information, see https://pre-commit.ci

murphp15 · 2024-04-22T06:10:34Z

...ipelines_control_service/src/main/java/com/vmware/taurus/service/deploy/JobImageBuilder.java

  @Value("${datajobs.git.url}")
  private String gitRepo;

+  @Value("${datajobs.git.cc.grc}")


why can't you just re use thw git url above ?

then you don't need the if stateent below ?

This is URL expected from Git Remote Code-commit tool, it is following format "codecommit::us-east-1://vdkdata-jobs" and only for this url format, git can fetch from AWS Code Commit repositories

Source - https://github.com/aws/git-remote-codecommit

yes but can't you just set this through {datajobs.git.url} property?

I tested that but code push through jgit didnt work in that case, so I included both the git and grc url, this is a optional property required only if datajobs.git.assumeIAMRole is true, maybe I can add a comment before this field in properties file to clarify this further

murphp15 · 2024-04-22T06:27:07Z

...ects/pipelines_control_service/src/main/java/com/vmware/taurus/service/upload/JobUpload.java

   * @return resource containing data job content in a zip format.
   */
  public Optional<Resource> getDataJob(String jobName) {
-    CredentialsProvider credentialsProvider = gitCredentialsProvider.getProvider();


you are changing the flow?
Are you sure provider doesn't need to be called before every operation?

The getProvider function generates a simple CredentialProvider based on username and password which we are supplying from helm/properties, so I concluded that it's OK to call it once in constructor, rather than every time -

versatile-data-kit/projects/control-service/projects/pipelines_control_service/src/main/java/com/vmware/taurus/service/upload/GitCredentialsProvider.java

Line 30 in 8c67726

}

Same for the AWS code commit credential provider, we just need roleARN at the starting which we supplied using datajobs.aws.roleArn property

murphp15 · 2024-04-22T06:28:10Z

...ects/pipelines_control_service/src/main/java/com/vmware/taurus/service/upload/JobUpload.java

    this.jobUploadAllowListValidator = jobUploadAllowListValidator;
    this.jobUploadFilterListValidator = jobUploadFilterListValidator;
+
+    if(assumeCodeCommitIAMRole){


I don't really like this.
I would prefer if you could create a single provider bean outside of the class and pass it in using spring.
Please see this PR as an exampel https://github.com/vmware/versatile-data-kit/pull/3269/files

Updated the code according to your reference

for more information, see https://pre-commit.ci

antoniivanov · 2024-04-26T07:52:03Z

Looks good to me.

@mivanov1988 is one who have worked al ot on job-builder so if he can take a look that would be a bonus.

How did you test it? You can reuse/modify the script to deploy the control service in your own kubernetes and make sure that it works - https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/cicd/deploy-testing-pipelines-service.sh

antoniivanov · 2024-04-26T07:47:51Z

...control-service/projects/pipelines_control_service/src/main/resources/application.properties

 datajobs.git.url=${GIT_URL}
+datajobs.git.cc.grc=${GIT_GRC_URL}


We can just document that git.url supports both code commit and normal git URL.

Seems unnecessary to have both.

antoniivanov · 2024-04-26T07:48:33Z

...control-service/projects/pipelines_control_service/src/main/resources/application.properties

+datajobs.git.cc.grc=${GIT_GRC_URL}
+
+# datajobs.git.assumeIAMRole tells the control-service if the Service Account pattern should be used for AWS CodeCommit.
+datajobs.git.assumeIAMRole=${DATAJOBS_CC_AWS_ASSUME_IAM_ROLE:false}


New variables would also need to be exposed and documented n https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/helm_charts/pipelines-control-service/values.yaml

At least there's where we've tried to sort of maintain documentation and list of control service configuration.

antoniivanov

Assuming testing passes. I am ok with merging it.

vmwclabot · 2024-05-16T18:45:53Z

@SurajViitk, VMware has approved your signed contributor license agreement.

Temp to test credentials for aws code commit

3ed9812

github-actions bot added the title needs formatting label Apr 15, 2024

SurajViitk changed the title ~~Draft: Temp to test credentials for aws code commit~~ DRAFT control-service: Temp to test credentials for aws code commit Apr 15, 2024

github-actions bot removed the title needs formatting label Apr 15, 2024

vsuraj and others added 2 commits April 18, 2024 18:47

Code commit integration

15d4c95

[pre-commit.ci] auto fixes from pre-commit.com hooks

eb1d5df

for more information, see https://pre-commit.ci

SurajViitk changed the title ~~DRAFT control-service: Temp to test credentials for aws code commit~~ DRAFT control-service: AWS Code commit integration Apr 18, 2024

vsuraj and others added 2 commits April 21, 2024 17:41

Code commit message builder integration

22d3f77

[pre-commit.ci] auto fixes from pre-commit.com hooks

a0d13f4

for more information, see https://pre-commit.ci

murphp15 reviewed Apr 22, 2024

View reviewed changes

vsuraj and others added 2 commits April 23, 2024 23:24

review comment

e074075

[pre-commit.ci] auto fixes from pre-commit.com hooks

8bef395

for more information, see https://pre-commit.ci

murphp15 approved these changes Apr 24, 2024

View reviewed changes

bean fix

505fb09

antoniivanov reviewed Apr 26, 2024

View reviewed changes

antoniivanov approved these changes Apr 26, 2024

View reviewed changes

		datajobs.git.url=${GIT_URL}
		datajobs.git.cc.grc=${GIT_GRC_URL}

Conversation

SurajViitk commented Apr 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vmwclabot commented Apr 15, 2024

Uh oh!

vmwclabot commented Apr 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antoniivanov commented Apr 26, 2024

Uh oh!

antoniivanov Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antoniivanov left a comment

Choose a reason for hiding this comment

Uh oh!

vmwclabot commented May 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SurajViitk commented Apr 15, 2024 •

edited

Loading

antoniivanov Apr 26, 2024 •

edited

Loading