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.

Sign up for GitHub

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

Jump to bottom

[MNEDC]Resolving: SonarCloud defects for hardcoded IP #280

Merged
merged 1 commit into from
Mar 5, 2021
Merged

[MNEDC]Resolving: SonarCloud defects for hardcoded IP #280

merged 1 commit into from
Mar 5, 2021

Conversation

sun-sharma
Copy link
Contributor

@sun-sharma sun-sharma commented Feb 25, 2021 •
edited

Signed-off-by: Sunchit Sharma sun.sharma@samsung.com

Description

  1. To avoid hardcoding of MNEDC server IP :
    • Generated a random virtual IP for the server of the type 10.[RANDOM].[RANDOM].1
  2. To avoid same Numbers being assigned on every run, Seed for the pseudo-random generator has been randomized according to current time
  3. In case of a clash between the Actual(Private) IP and the Virtual IP a new random Virtual IP had been generated.
  4. Fixed the Test cased and Generated new mocks as Function Signatures have changed

Fixes #273

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  1. Two systems were set up : One in main NAT other in sub NAT
  2. Main NAT system got a random virtual IP which was of the type 10...1
  3. Sub NAT system got the same IP with 2 at the 4th place
  4. Requests were sent from both system and they worked fine

#Logs

Main System

INFO[2021-02-25T12:21:43Z]server.go:390 generateServerIP Virtual IP : 10.195.226.1
INFO[2021-02-25T12:21:43Z]server.go:391 generateServerIP Private IP : 192.168.1.29

Sub System

INFO[2021-02-25T12:22:47Z]networkhelper.go:129 [networkmgr] returning 10.195.226.2

Successively, in all runs different IPs were assigned.

Example
1. Describe the reproduction procedures freely.
2. Or list up the test description like :
  - [X] Unittest
  - [X] Execution of Container
  - [X] Execution on top of Native

Test environment configuration:

  • Firmware version: Ubuntu 16.04
  • Hardware: x86-64
  • Toolchain: Go 15.1
  • Edge Orchestration Release: Coconut

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@sun-sharma sun-sharma changed the title Sonar final [MNEDC]Resolving: SonarCloud defects for hardcoded IP Feb 25, 2021
@tiokim tiokim added bug Something isn't working high priority It should be resolved ASAP labels Feb 26, 2021
tiokim
tiokim reviewed Feb 26, 2021
View reviewed changes
Copy link
Contributor

@tiokim tiokim left a comment

Choose a reason for hiding this comment

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

Thank you very much for your efforts!
Please take a look at my comments.

internal/controller/discoverymgr/mnedc/server/mocks/mocks_server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/mocks/mocks_server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
internal/controller/discoverymgr/mnedc/server/server.go Outdated Show resolved Hide resolved
@MoonkiHong MoonkiHong added this to In progress in MNEDC Mar 2, 2021
MoonkiHong
MoonkiHong reviewed Mar 2, 2021
View reviewed changes
Copy link
Contributor

@MoonkiHong MoonkiHong left a comment

Choose a reason for hiding this comment

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

Please refer to my comment! 😄

internal/controller/discoverymgr/mnedc/server/mocks/mocks_server.go Outdated Show resolved Hide resolved
@sun-sharma sun-sharma force-pushed the Sonar-Final branch 2 times, most recently from 81c590e to 85f88a2 Compare March 2, 2021 14:17
@sun-sharma
Copy link
Contributor Author

Thanks for the review! Pushed the review changes, PTAL.

@sun-sharma
[MNEDC] Resolving : SonarCloud defects for hardcoded IP
7a7cb50 
1. To avoid hardcoding of MNEDC server IP :
   - Generated a random virtual IP for the server of the type 10.<RANDOM>.<RANDOM>.1
2. To avoid same Numbers being assigned on every run, Seed for the pseudo-random generator has been randomized according to current time
3. In case of a clash between the Actual(Private) IP and the Virtual IP a new random Virtual IP had been generated.
4. Fixed the Test cased and Generated new mocks as Function Signatures have changed

Signed-off-by: Sunchit Sharma <sun.sharma@samsung.com>
@sonarcloud
Copy link

sonarcloud bot commented Mar 3, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

tiokim
tiokim approved these changes Mar 3, 2021
View reviewed changes
Copy link
Contributor

@tiokim tiokim left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

MNEDC automation moved this from In progress to Reviewer Approved Mar 3, 2021
suresh-lc
suresh-lc approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@suresh-lc suresh-lc left a comment

Choose a reason for hiding this comment

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

LGTM

MoonkiHong
MoonkiHong approved these changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@MoonkiHong MoonkiHong left a comment

Choose a reason for hiding this comment

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

+1

@MoonkiHong MoonkiHong merged commit a0ac6b3 into lf-edge:master Mar 5, 2021
MNEDC automation moved this from Reviewer Approved to Done Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiokim tiokim tiokim approved these changes

@MoonkiHong MoonkiHong MoonkiHong approved these changes

@tdrozdovsky tdrozdovsky tdrozdovsky approved these changes

@suresh-lc suresh-lc suresh-lc approved these changes

@mgjeong mgjeong Awaiting requested review from mgjeong

Assignees
No one assigned
Labels
bug Something isn't working high priority It should be resolved ASAP
Projects
MNEDC
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MNEDC] Security Hotspots detected by SonarCloud
5 participants
@sun-sharma @tiokim @MoonkiHong @tdrozdovsky @suresh-lc