Skip to content

Conversation

@gabrielm-splunk
Copy link
Collaborator

@gabrielm-splunk gabrielm-splunk commented Sep 10, 2025

Description

Upgrading operator-sdk to version 1.41.1 and Golang version to 1.24.4. There were some linting changes that trickled through, but were very minor, so just decided to leave in. Thankfully, there weren't any breaking changes, so this was pretty straightforward other than some go mod changes (ran go mod tidy to resolve). This will help resolve security vulnerability: https://splunk.atlassian.net/browse/VULN-42799

Key Changes

The main updates are in the .env and Dockerfile

Testing and Verification

Ran operator locally, installed operator on EKS cluster, and ran unit tests. Added this branch to be picked up by integration testing, so will monitor outputs of that.

Related Issues

PR Checklist

  • Code changes adhere to the project's coding standards.
  • Relevant unit and integration tests are included.
  • Documentation has been updated accordingly.
  • All tests pass locally.
  • The PR description follows the project's guidelines.

@gabrielm-splunk
Copy link
Collaborator Author

I see the check for biased language is failing here, any insights on how to solve? There were very minimal changes and the only change that's included that has the biased word master is in the deps for Masterminds

@coveralls
Copy link
Collaborator

coveralls commented Sep 10, 2025

Pull Request Test Coverage Report for Build 19141739219

Details

  • 8 of 8 (100.0%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 86.561%

Totals Coverage Status
Change from base Build 19137368717: 0.009%
Covered Lines: 10718
Relevant Lines: 12382

💛 - Coveralls

@rlieberman-splunk
Copy link
Collaborator

I see the check for biased language is failing here, any insights on how to solve? There were very minimal changes and the only change that's included that has the biased word master is in the deps for Masterminds

We can add the go.mod file to https://github.com/splunk/splunk-operator/blob/main/.biased_lang_exclude. go.sum is already excluded. There's more information here if you need it too! https://github.com/splunk/biased-lang-linter/pkgs/container/biased_lang#excluding-directories-and-files

@gabrielm-splunk
Copy link
Collaborator Author

gabrielm-splunk commented Sep 11, 2025

I see the check for biased language is failing here, any insights on how to solve? There were very minimal changes and the only change that's included that has the biased word master is in the deps for Masterminds

We can add the go.mod file to https://github.com/splunk/splunk-operator/blob/main/.biased_lang_exclude. go.sum is already excluded. There's more information here if you need it too! https://github.com/splunk/biased-lang-linter/pkgs/container/biased_lang#excluding-directories-and-files

Got it, done @rlieberman-splunk

@kasiakoziol
Copy link
Collaborator

https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.40.0/:

https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.41.0/

- develop
- main
- feature**
- CSPL-3964-upgrade-operator-sdk
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remember to remove it before merge.

@gabrielm-splunk
Copy link
Collaborator Author

https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.40.0/:

https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.41.0/

@kasiakoziol Yeah, I missed some of these. I was looking at a wrong version of the upgrade-sdk-version doc, will address today

@gabrielm-splunk gabrielm-splunk force-pushed the CSPL-3964-upgrade-operator-sdk branch from a4416d1 to a572a7d Compare September 25, 2025 07:38
@gabrielm-splunk gabrielm-splunk force-pushed the CSPL-3964-upgrade-operator-sdk branch from a572a7d to cb364e3 Compare October 1, 2025 06:13
@gabrielm-splunk gabrielm-splunk force-pushed the CSPL-3964-upgrade-operator-sdk branch from cb364e3 to 48a17ca Compare October 10, 2025 07:33
Copy link
Collaborator

@patrykw-splunk patrykw-splunk left a comment

Choose a reason for hiding this comment

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

Open question, does this MR should include certificate watcher changes?
I'm also wondering if Cert-manager specific resources from the upgrade guide should be included here.

Here is a reference
https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.40.0/

@gabrielm-splunk
Copy link
Collaborator Author

Open question, does this MR should include certificate watcher changes? I'm also wondering if Cert-manager specific resources from the upgrade guide should be included here.

Here is a reference https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.40.0/

We discussed this offline and determined that we will handle cert-manager-specific changes in a follow-up PR

@gabrielm-splunk gabrielm-splunk force-pushed the CSPL-3964-upgrade-operator-sdk branch from 722fb07 to 42cfdc0 Compare October 29, 2025 14:17

// Override script locations to use absolute paths for integration tests
// These are relative to test/appframework_aws/s1/, so we go up 3 levels to project root
enterprise.GetReadinessScriptLocation = func() string {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Although not fully ideal, I saw precedent for doing this for examples here and here so felt more comfortable with this (test was failing trying to run these scripts prior to this)

untarredAppRootFolder := string(bytepath)
untarredAppRootFolder = untarredAppRootFolder[:len(untarredAppRootFolder)-1] //removing \n at the end of folder path
// Use filepath.ReadDir to reliably get the first directory
entries, err := os.ReadDir(untarredCurrentAppFolder)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Description on why these changes were necessary:

Sometimes the wildcard parsing here would cause us to not be able to properly pick up the first directory, causing the disabled app changes to not be processed sometimes. This change uses Go idiomatic way to do the same thing in a more reliable way.

return nil, fmt.Errorf("kubeClient is nil")
}

// Register controllers with the manager
Copy link
Collaborator Author

@gabrielm-splunk gabrielm-splunk Oct 29, 2025

Choose a reason for hiding this comment

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

Not 100% sure why this is now necessary since the upgrade, but the Monitoring Console would hang while waiting to become ready without this "manual" controller registration

@kasiakoziol
Copy link
Collaborator

@gabrielm-splunk If you want to upgrade to 1.34, you have to pull my changes from develop because that's not only a one line change.

@gabrielm-splunk
Copy link
Collaborator Author

@gabrielm-splunk If you want to upgrade to 1.34, you have to pull my changes from develop because that's not only a one line change.

@kasiakoziol I did rebase off of develop, so it does contain those changes, unless you pushed within the last couple of hours

@gabrielm-splunk gabrielm-splunk force-pushed the CSPL-3964-upgrade-operator-sdk branch from 93141e5 to 4d01194 Compare November 6, 2025 15:59
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.

6 participants