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

[infra-proxy-service] Add integration testing for infra-proxy-service #4690

Merged
merged 16 commits into from
Feb 10, 2021

Conversation

kvivek1115
Copy link

@kvivek1115 kvivek1115 commented Feb 5, 2021

🔩 Description: What code changed, and why?

  • Add integration test setup for infra-proxy-service.
  • Added GetClients and GetClient test cases for reference. Have put chef-repo data using scripts, once we have create/update/delete API ready will manage the data using APIs.

⛓️ Related Resources

NA

👍 Definition of Done

  • infra_service_integration hab command available for integration testing.

👟 How to Build and Test the Change

  • Steps to test:
    • start_all_service # if not already started
    • source .studio/infra-proxy-service
    • To run the test cases run in hab infra_service_integration

✅ Checklist

📷 Screenshots, if applicable

Signed-off-by: Vivek Singh [email protected]

Aha! Link: https://chef.aha.io/epics/SH-E-443

@netlify
Copy link

netlify bot commented Feb 5, 2021

Deploy preview for chef-automate processing.

Building with commit 0d3c4c7

https://app.netlify.com/sites/chef-automate/deploys/6023e4052f28ce000733e4f9

@kvivek1115 kvivek1115 added this to the Automate 2021 Q1 SP2 milestone Feb 9, 2021
@kvivek1115 kvivek1115 marked this pull request as ready for review February 9, 2021 13:29
@kalroy kalroy self-requested a review February 9, 2021 15:02
res, err := infraProxy.GetClients(ctx, req)
assert.NoError(t, err)
assert.NotNil(t, res)
assert.Equal(t, 3, len(res.GetClients()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

These tests assume that we have a predefined test data loaded to the server. This is fine at this point since we do not have the API to add the data. But this needs to be addressed later.

res, err := infraProxy.GetClient(ctx, req)
assert.NoError(t, err)
assert.NotNil(t, res)
assert.Equal(t, "chef-load-1", res.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

}

// GlobalTeardown is the place where you tear everything down after we have finished
func (s *Suite) GlobalTeardown() {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please confirm if we need to close any open connections like authn or authz.

Copy link
Author

Choose a reason for hiding this comment

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

As I can check in this method we have performed some data cleanup like Postgres data, elastic search indices, etc, not the service connections. will investigate more on this!
Thanks!

@kalroy kalroy force-pushed the VSingh/infra-proxy-service-integration-test branch from d30050f to e17ba8a Compare February 9, 2021 16:04
Vivek Singh and others added 4 commits February 10, 2021 13:37
@kalroy kalroy merged commit 09121a6 into master Feb 10, 2021
@kalroy kalroy deleted the VSingh/infra-proxy-service-integration-test branch February 10, 2021 14:31
// Adding the subjects if missing if gRPC calls from internal service level
subjects := auth_context.FromContext(auth_context.FromIncomingMetadata(ctx)).Subjects
if len(subjects) == 0 {
subjects = []string{"tls:service:compliance-service:internal"}
Copy link
Author

Choose a reason for hiding this comment

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

it should be tls:service:deployment-service:internal typo here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants