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

No "global" exec runner instance inside cloudfoundry pkg #1771

Merged
merged 2 commits into from
Jul 10, 2020
Merged

No "global" exec runner instance inside cloudfoundry pkg #1771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e24d442
Dont work upon a global command.Command instance inside cloudfoundry …
marcusholl Jul 7, 2020
1b29a01
Merge remote-tracking branch 'github/master' into HEAD
marcusholl Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/abapEnvironmentRunATCCheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@ func checkHost(config abapEnvironmentRunATCCheckOptions, details connectionDetai
return details, errors.New("Parameters missing. Please provide EITHER the Host of the ABAP server OR the Cloud Foundry ApiEndpoint, Organization, Space, Service Instance and a corresponding Service Key for the Communication Scenario SAP_COM_0510")
}
var abapServiceKey cloudfoundry.ServiceKey
abapServiceKey, err = cloudfoundry.ReadServiceKeyAbapEnvironment(cfconfig, true)
cf := cloudfoundry.CFUtils{Exec: &command.Command{}}
abapServiceKey, err = cf.ReadServiceKeyAbapEnvironment(cfconfig, true)
if err != nil {
return details, fmt.Errorf("Reading Service Key failed: %w", err)
}
Expand Down
62 changes: 51 additions & 11 deletions pkg/cloudfoundry/Authentication.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,45 @@ import (
"github.com/SAP/jenkins-library/pkg/log"
)

var c command.ExecRunner = &command.Command{}

//LoginCheck checks if user is logged in to Cloud Foundry.
//If user is not logged in 'cf api' command will return string that contains 'User is not logged in' only if user is not logged in.
//If the returned string doesn't contain the substring 'User is not logged in' we know he is logged in.
func LoginCheck(options LoginOptions) (bool, error) {
func (cf *CFUtils) LoginCheck(options LoginOptions) (bool, error) {
var err error

_c := cf.Exec

if _c == nil {
_c = &command.Command{}
}

if options.CfAPIEndpoint == "" {
return false, errors.New("Cloud Foundry API endpoint parameter missing. Please provide the Cloud Foundry Endpoint")
}

//Check if logged in --> Cf api command responds with "not logged in" if positive
var cfCheckLoginScript = append([]string{"api", options.CfAPIEndpoint}, options.CfAPIOpts...)

defer func() {
// We set it back to what is set from the generated stub. Of course this is not
// fully accurate in case we create our own instance above (nothing handed in via
// the receiver).
// Would be better to remember the old stdout and set back to this.
// But command.Command does not allow to get the currently set
// stdout handler.
// Reason for changing the output stream here: we need to parse the output
// of the command issued here in order to check if we are already logged in.
// This is expected to change soon to a boolean variable where we remember the
// login state.
_c.Stdout(log.Writer())
}()

var cfLoginBytes bytes.Buffer
c.Stdout(&cfLoginBytes)
_c.Stdout(&cfLoginBytes)
Copy link
Member

Choose a reason for hiding this comment

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

Does this make stdout inmutable for LoginCheck function?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I already spottet that too. We have to reset the old stdout afterwards. But at the moment it is not possible to retrieve the current stdout via our wrapper. Needs to be added. At least the underlying cmd allow to get the currently set stream(s).

The point is: we have to ensure that the "cmdRunner" is not permanently changed inside the func. I had this already. As a result logging output stopped after calling login. Of course also for the subsequent cf deploy.

Copy link
Member

@dominiklendle dominiklendle Jul 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I would propose to finalize on discussion in #1760 . This probably solves this whole problem and we can leave the whole function out


var result string

err = c.RunExecutable("cf", cfCheckLoginScript...)
err = _c.RunExecutable("cf", cfCheckLoginScript...)

if err != nil {
return false, fmt.Errorf("Failed to check if logged in: %w", err)
Expand All @@ -52,17 +70,22 @@ func LoginCheck(options LoginOptions) (bool, error) {

//Login logs user in to Cloud Foundry via cf cli.
//Checks if user is logged in first, if not perform 'cf login' command with appropriate parameters
func Login(options LoginOptions) error {

func (cf *CFUtils) Login(options LoginOptions) error {
var err error

_c := cf.Exec

if _c == nil {
_c = &command.Command{}
}

if options.CfAPIEndpoint == "" || options.CfOrg == "" || options.CfSpace == "" || options.Username == "" || options.Password == "" {
return fmt.Errorf("Failed to login to Cloud Foundry: %w", errors.New("Parameters missing. Please provide the Cloud Foundry Endpoint, Org, Space, Username and Password"))
}

var loggedIn bool

loggedIn, err = LoginCheck(options)
loggedIn, err = cf.LoginCheck(options)

if loggedIn == true {
return err
Expand All @@ -82,7 +105,7 @@ func Login(options LoginOptions) error {

log.Entry().WithField("cfAPI:", options.CfAPIEndpoint).WithField("cfOrg", options.CfOrg).WithField("space", options.CfSpace).Info("Logging into Cloud Foundry..")

err = c.RunExecutable("cf", cfLoginScript...)
err = _c.RunExecutable("cf", cfLoginScript...)
}

if err != nil {
Expand All @@ -94,12 +117,19 @@ func Login(options LoginOptions) error {

//Logout logs User out of Cloud Foundry
//Logout can be perforned via 'cf logout' command regardless if user is logged in or not
func Logout() error {
func (cf *CFUtils) Logout() error {

_c := cf.Exec

if _c == nil {
_c = &command.Command{}
}

var cfLogoutScript = "logout"

log.Entry().Info("Logging out of Cloud Foundry")

err := c.RunExecutable("cf", cfLogoutScript)
err := _c.RunExecutable("cf", cfLogoutScript)
if err != nil {
return fmt.Errorf("Failed to Logout of Cloud Foundry: %w", err)
}
Expand All @@ -117,3 +147,13 @@ type LoginOptions struct {
CfAPIOpts []string
CfLoginOpts []string
}

// CFUtils ...
type CFUtils struct {
// In order to avoid clashes between parallel workflows requiring cf login/logout
// this instance of command.ExecRunner can be configured accordingly by settings the
// environment variables CF_HOME to distict directories.
// In order to ensure plugins installed to the cf cli are found environment variables
// CF_PLUGIN_HOME can be set accordingly.
Exec command.ExecRunner
}
41 changes: 21 additions & 20 deletions pkg/cloudfoundry/CloudFoundry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ func TestCloudFoundryLoginCheck(t *testing.T) {

m := &mock.ExecMockRunner{}

defer func() {
c = &command.Command{}
}()
c = m

t.Run("CF Login check: missing endpoint parameter", func(t *testing.T) {
cfconfig := LoginOptions{}
loggedIn, err := LoginCheck(cfconfig)
cf := CFUtils{Exec: m}
loggedIn, err := cf.LoginCheck(cfconfig)
assert.False(t, loggedIn)
assert.EqualError(t, err, "Cloud Foundry API endpoint parameter missing. Please provide the Cloud Foundry Endpoint")
})
Expand All @@ -38,7 +34,8 @@ func TestCloudFoundryLoginCheck(t *testing.T) {
cfconfig := LoginOptions{
CfAPIEndpoint: "https://api.endpoint.com",
}
loggedIn, err := LoginCheck(cfconfig)
cf := CFUtils{Exec: m}
loggedIn, err := cf.LoginCheck(cfconfig)
assert.False(t, loggedIn)
assert.Error(t, err)
assert.Equal(t, []mock.ExecCall{mock.ExecCall{Exec: "cf", Params: []string{"api", "https://api.endpoint.com"}}}, m.Calls)
Expand All @@ -51,7 +48,8 @@ func TestCloudFoundryLoginCheck(t *testing.T) {
cfconfig := LoginOptions{
CfAPIEndpoint: "https://api.endpoint.com",
}
loggedIn, err := LoginCheck(cfconfig)
cf := CFUtils{Exec: m}
loggedIn, err := cf.LoginCheck(cfconfig)
if assert.NoError(t, err) {
assert.True(t, loggedIn)
assert.Equal(t, []mock.ExecCall{mock.ExecCall{Exec: "cf", Params: []string{"api", "https://api.endpoint.com"}}}, m.Calls)
Expand All @@ -67,7 +65,9 @@ func TestCloudFoundryLoginCheck(t *testing.T) {
// should never used in productive environment, but it is useful for rapid prototyping/troubleshooting
CfAPIOpts: []string{"--skip-ssl-validation"},
}
loggedIn, err := LoginCheck(cfconfig)

cf := CFUtils{Exec: m}
loggedIn, err := cf.LoginCheck(cfconfig)
if assert.NoError(t, err) {
assert.True(t, loggedIn)
assert.Equal(t, []mock.ExecCall{
Expand All @@ -86,17 +86,13 @@ func TestCloudFoundryLogin(t *testing.T) {

m := &mock.ExecMockRunner{}

defer func() {
c = &command.Command{}
}()
c = m

t.Run("CF Login: missing parameter", func(t *testing.T) {

defer loginMockCleanup(m)

cfconfig := LoginOptions{}
err := Login(cfconfig)
cf := CFUtils{Exec: m}
err := cf.Login(cfconfig)
assert.EqualError(t, err, "Failed to login to Cloud Foundry: Parameters missing. Please provide the Cloud Foundry Endpoint, Org, Space, Username and Password")
})
t.Run("CF Login: failure", func(t *testing.T) {
Expand All @@ -114,7 +110,8 @@ func TestCloudFoundryLogin(t *testing.T) {
Password: "testPassword",
}

err := Login(cfconfig)
cf := CFUtils{Exec: m}
err := cf.Login(cfconfig)
if assert.EqualError(t, err, "Failed to login to Cloud Foundry: wrong password or account does not exist") {
assert.Equal(t, []mock.ExecCall{
mock.ExecCall{Exec: "cf", Params: []string{"api", "https://api.endpoint.com"}},
Expand Down Expand Up @@ -143,7 +140,8 @@ func TestCloudFoundryLogin(t *testing.T) {
Username: "testUser",
Password: "testPassword",
}
err := Login(cfconfig)
cf := CFUtils{Exec: m}
err := cf.Login(cfconfig)
if assert.NoError(t, err) {
assert.Equal(t, []mock.ExecCall{
mock.ExecCall{Exec: "cf", Params: []string{"api", "https://api.endpoint.com"}},
Expand Down Expand Up @@ -180,7 +178,8 @@ func TestCloudFoundryLogin(t *testing.T) {
"--skip-ssl-validation",
},
}
err := Login(cfconfig)
cf := CFUtils{Exec: m}
err := cf.Login(cfconfig)
if assert.NoError(t, err) {
assert.Equal(t, []mock.ExecCall{
mock.ExecCall{Exec: "cf", Params: []string{
Expand All @@ -206,7 +205,8 @@ func TestCloudFoundryLogin(t *testing.T) {

func TestCloudFoundryLogout(t *testing.T) {
t.Run("CF Logout", func(t *testing.T) {
err := Logout()
cf := CFUtils{Exec: &mock.ExecMockRunner{}}
err := cf.Logout()
if err == nil {
assert.Equal(t, nil, err)
} else {
Expand All @@ -227,7 +227,8 @@ func TestCloudFoundryReadServiceKeyAbapEnvironment(t *testing.T) {
Password: "testPassword",
}
var abapKey ServiceKey
abapKey, err := ReadServiceKeyAbapEnvironment(cfconfig, true)
cf := CFUtils{Exec: &command.Command{}}
abapKey, err := cf.ReadServiceKeyAbapEnvironment(cfconfig, true)
assert.Equal(t, "", abapKey.Abap.Password)
assert.Equal(t, "", abapKey.Abap.Username)
assert.Equal(t, "", abapKey.Abap.CommunicationArrangementID)
Expand Down
23 changes: 15 additions & 8 deletions pkg/cloudfoundry/Services.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,21 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"

"github.com/SAP/jenkins-library/pkg/command"
"github.com/SAP/jenkins-library/pkg/log"
"strings"
)

//ReadServiceKeyAbapEnvironment from Cloud Foundry and returns it.
//Depending on user/developer requirements if he wants to perform further Cloud Foundry actions the cfLogoutOption parameters gives the option to logout after reading ABAP communication arrangement or not.
func ReadServiceKeyAbapEnvironment(options ServiceKeyOptions, cfLogoutOption bool) (ServiceKey, error) {
func (cf *CFUtils) ReadServiceKeyAbapEnvironment(options ServiceKeyOptions, cfLogoutOption bool) (ServiceKey, error) {
Copy link

Choose a reason for hiding this comment

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

Method CFUtils.ReadServiceKeyAbapEnvironment has 51 lines of code (exceeds 50 allowed). Consider refactoring.


_c := cf.Exec

if _c == nil {
_c = &command.Command{}
}

var abapServiceKey ServiceKey
var err error

Expand All @@ -25,16 +32,16 @@ func ReadServiceKeyAbapEnvironment(options ServiceKeyOptions, cfLogoutOption boo
Password: options.Password,
}

err = Login(config)
err = cf.Login(config)
var serviceKeyBytes bytes.Buffer
c.Stdout(&serviceKeyBytes)
_c.Stdout(&serviceKeyBytes)
if err == nil {
//Reading Service Key
log.Entry().WithField("cfServiceInstance", options.CfServiceInstance).WithField("cfServiceKey", options.CfServiceKey).Info("Read service key for service instance")

cfReadServiceKeyScript := []string{"service-key", options.CfServiceInstance, options.CfServiceKey}

err = c.RunExecutable("cf", cfReadServiceKeyScript...)
err = _c.RunExecutable("cf", cfReadServiceKeyScript...)
}
if err == nil {
var serviceKeyJSON string
Expand All @@ -54,7 +61,7 @@ func ReadServiceKeyAbapEnvironment(options ServiceKeyOptions, cfLogoutOption boo
if err != nil {
if cfLogoutOption == true {
var logoutErr error
logoutErr = Logout()
logoutErr = cf.Logout()
if logoutErr != nil {
return abapServiceKey, fmt.Errorf("Failed to Logout of Cloud Foundry: %w", err)
}
Expand All @@ -65,7 +72,7 @@ func ReadServiceKeyAbapEnvironment(options ServiceKeyOptions, cfLogoutOption boo
//Logging out of CF
if cfLogoutOption == true {
var logoutErr error
logoutErr = Logout()
logoutErr = cf.Logout()
if logoutErr != nil {
return abapServiceKey, fmt.Errorf("Failed to Logout of Cloud Foundry: %w", err)
}
Expand Down